Skip to content

fix(test): update stale NIM outage-domain test to match #941's independent-account contract - #945

Merged
seonghobae merged 1 commit into
mainfrom
fix/stale-nim-outage-domain-test
Aug 31, 2026
Merged

seonghobae merged 1 commit into
mainfrom
fix/stale-nim-outage-domain-test

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • tests/test_discovery_bootstrap_selection.py::test_bootstrap_selector_treats_nim_primary_and_sub_as_one_outage_domain fails unconditionally on current main (verified on a clean checkout). It asserts the pre-fix(discovery): keep credential catalogs independent #941 behavior of select_bootstrap_discovered_agents — that nvidia_nim/nvidia_nim_sub collapse into one provider family so only one of the two can be selected alongside another provider.
  • Commit 5224a4c4 (part of fix(discovery): keep credential catalogs independent #941, "codex/all-key-discovery-latency-routing") deliberately removed that grouping from the implementation and documented the reversal in docs/product-technical-gap-baseline.md and ADR 0032: "this historical provider-family conclusion is no longer the product contract... every credential account is discovered and judged independently" — because each NIM API key (NVIDIA_NIM_API_KEY vs NVIDIA_NIM_API_KEY_SUB) can be entitled to a different model catalog and must not be assumed identical or collapsible. That commit updated the implementation and both docs but never touched this one test in a different test file, leaving it asserting the now-superseded behavior — red on main ever since.
  • This PR updates only the test (no production code change) to assert the current, documented, already-shipped contract: with nvidia_nim and nvidia_nim_sub as the two cheapest candidates and limit=2, both are now selected ([nim_primary, nim_sub]), matching how every other independent provider pair in this file already behaves.

Developer experience

No API or behavior change — production code is untouched. select_bootstrap_discovered_agents's own docstring already states "No vendor or endpoint name is used to infer a shared family or collapse credential state"; this test now matches that docstring instead of contradicting it.

User experience

None — test-only change.

Test plan

  • Reproduced the failure on a clean origin/main checkout (git worktree add + pytest tests/test_discovery_bootstrap_selection.py::test_bootstrap_selector_treats_nim_primary_and_sub_as_one_outage_domain) to confirm it is pre-existing and unrelated to any other in-flight change.
  • python -m pytest tests/test_discovery_bootstrap_selection.py -q — 14/14 pass after the fix.
  • interrogate (project-wide, fail-under = 80): 100%.

Generated by Claude Code


Devin Review

…ndent-account contract

test_bootstrap_selector_treats_nim_primary_and_sub_as_one_outage_domain
asserted the pre-#941 behavior (select_bootstrap_discovered_agents
collapsing nvidia_nim/nvidia_nim_sub into one provider family). 5224a4c
(part of #941) deliberately removed that grouping -- documented in
docs/product-technical-gap-baseline.md and ADR 0032 as superseded,
since each credential's API key may be entitled to a different model
catalog -- but left this one test asserting the old behavior, making it
fail unconditionally on main. Verified this fails identically on a
clean origin/main checkout with none of this PR's changes present.

No production code changed; the test now asserts the documented,
already-shipped independent-account contract.
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 42 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ed58a1d6-9d00-433a-a18b-f3c610219275

📥 Commits

Reviewing files that changed from the base of the PR and between 42da1d5 and bffbc72.

📒 Files selected for processing (1)
  • tests/test_discovery_bootstrap_selection.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

Copy link
Copy Markdown
Contributor Author

opencode-review failed the same way it does on every fresh push in this org: opencode-review-target in .github's central opencode-review.yml is a fail-closed gate that only passes once opencode-agent has posted an APPROVED/CHANGES_REQUESTED review on this exact head SHA (dispatched separately via repository_dispatch, driven by noema-review, which completed successfully here — the actual review verdict lands asynchronously after that). Not caused by this PR's diff; no fix belongs here. Keeping this PR watched until the scheduler's retry lands a verdict and this goes green, or until other CI/review evidence is sufficient.


Generated by Claude Code

@seonghobae
seonghobae merged commit 0adca47 into main Aug 31, 2026
30 of 31 checks passed
@seonghobae
seonghobae deleted the fix/stale-nim-outage-domain-test branch August 31, 2026 03:22
seonghobae pushed a commit that referenced this pull request Aug 31, 2026
Brings in #945's already-merged, authoritative fix for the stale
test_bootstrap_selector_treats_nim_primary_and_sub_as_one_outage_domain
(main @ bffbc72, merged via 0adca47) instead of keeping this branch's own
port of the equivalent #947 fix, which #945 beat to main and #947 was then
closed as superseded. Conflict in tests/test_discovery_bootstrap_selection.py
resolved by taking main's version of that one test function entirely
(git checkout --theirs) rather than reconciling two different renames of the
same fix. Added a short "Update" note to this branch's earlier
docs/product-technical-gap-baseline.md entry recording that #945 landed
first; no other file conflicted.

tests/test_discovery_bootstrap_selection.py -q: 14 passed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants