Skip to content

docs: correct stale direct-NIM claims after contextual-orchestrator migration - #1527

Closed
seonghobae wants to merge 1 commit into
mainfrom
docs/nim-gateway-doctoring-accuracy
Closed

docs: correct stale direct-NIM claims after contextual-orchestrator migration#1527
seonghobae wants to merge 1 commit into
mainfrom
docs/nim-gateway-doctoring-accuracy

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Summary

Documentation-only accuracy fix, found while surveying this cycle's goal (route Noema/OpenCode/Strix through contextual-orchestrator's orchestrator/free, eliminate direct NIM communication). That survey confirmed OpenCode review, Strix, Noema-review, and the hourly PR-review-autofix worker are already fully routed through the vendored contextual-orchestrator gateway — no live direct-NIM HTTP call exists anywhere in this repo's workflows. But three documents still described the write-capable autofix worker's retired direct-NIM architecture (NVIDIA_API_KEY bound to a hard-coded mistralai/mistral-small-4-119b-2603) as current fact, contradicting the actual .github/workflows/pr-review-autofix.yml and its own contract test (test_scheduled_autofix_routes_through_contextual_orchestrator, which explicitly forbids https://integrate.api.nvidia.com/v1 and "enabled_providers": ["nvidia-nim"]). This is the exact "follow-up doc cleanup, not attempted this pass" gap already flagged in docs/product-technical-gap-baseline.md.

Changes

  • docs/doctoring/hourly-nvidia-nim-autofix.md: added a "Status (2026-08-31 correction)" section pointing at ADR-0003; rewrote "Provider contract", "Credential boundary", and "Rollback" to describe the current gateway/sidecar architecture (five KV-registered provider secrets feeding auto-discovery, never one bound directly); corrected two present-tense claims in "Verification contract" and "Scheduling and activation". Historical TDD-evidence records (exact-head RED/GREEN commit SHAs) are left untouched as dated history, per this repo's "append a dated note, don't rewrite history" convention. Removed three now-orphaned APA references to NVIDIA-specific model docs whose claims no longer appear in the body.
  • docs/automation/hourly-review-repair.md: corrected the caller-contract prose (was: "uses OpenCode with NVIDIA NIM"; "the model credential is scoped exclusively to the two OpenCode execution steps") and the operator/verification checklists to describe five gateway provider secrets scoped to the sidecar-provisioning step, not a single directly-bound NVIDIA credential.
  • ARCHITECTURE.md: renamed "Hourly NVIDIA NIM repair gate" → "Hourly contextual-orchestrator repair gate", updated its Mermaid diagram's NIM node to Gateway["contextual-orchestrator sidecar: orchestrator/free"], and corrected the "Control-plane data flow" sequence diagram and "Trust boundaries" bullet that both stated agents bind NVIDIA_NIM_API_KEY directly.
  • CHANGELOG.md: ## [Unreleased] entry summarizing the correction.

Verification

  • Targeted: python3 -m pytest tests/test_pr_review_autofix_nvidia_nim_contract.py tests/test_pr_review_fix_hourly_contract.py -q40 passed.
  • Full suite: coverage run -m pytest tests && coverage report --show-missing && interrogate2126 passed, 1 skipped; coverage 100% (10543/10543 statements, 4228/4228 branches); interrogate 100%.
  • Verified all contract-test-pinned substrings/anti-substrings in docs/doctoring/hourly-nvidia-nim-autofix.md survive the edit (ordinary and conflict repair, including ignored paths, `.git` and `.git/*`, `core.hooksPath=/dev/null`, explicit revalidated repository URL, the two forbidden legacy phrases, and the three required citations).
  • git diff --check: clean.

No workflow, script, or test behavior changes — documentation only.


Generated by Claude Code

…igration

ARCHITECTURE.md's "Hourly NVIDIA NIM repair gate" diagram,
docs/doctoring/hourly-nvidia-nim-autofix.md, and
docs/automation/hourly-review-repair.md still described the write-capable
PR-review-autofix worker binding NVIDIA NIM directly (NVIDIA_API_KEY bound to
a hard-coded mistralai/mistral-small-4-119b-2603). The actual workflow and
its contract test (test_scheduled_autofix_routes_through_contextual_orchestrator)
already require routing through the vendored contextual-orchestrator sidecar
and the contextual-orchestrator/orchestrator/free virtual model id, per
ADR-0003 and the org's 2026-08-18 gateway decision. Corrected the present-tense
provider/credential claims in all three documents; historical TDD-evidence
records (exact-head RED/GREEN commit SHAs) are left as dated history.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

Copy link
Copy Markdown
Contributor Author

exact-head-path-policy failed on this head (0c7803d3), job 99613849081: test_strix_quick_gate: 1 failure(s)FAIL: opencode required workflow bootstrap must not depend on required-workflow event payload fields.

Not this PR's fault (this PR only touches ARCHITECTURE.md, CHANGELOG.md, and two docs/ files — nothing under .github/workflows/ or scripts/ci/). Root-caused it: scripts/ci/test_strix_quick_gate.sh:525's awk '/^ required-workflow-bootstrap:$/,/^[^ ]/' range has always had a boundary bug — /^[^ ]/ only matches a fully-unindented line, so it never stops at the next 2-space-indented job header and instead sweeps in every subsequent job in opencode-review.yml (coverage-source-tree, coverage-evidence, opencode-review-target) until end of file. That bug was latent until today's #1497 ("require substantive agent verdicts") added a genuinely new step-level if: github.event.action != 'closed' inside opencode-review-target — which the over-broad awk range now sweeps in and misattributes to required-workflow-bootstrap, even though that job itself has no if: at all and is fully compliant. Confirmed reproducing identically against a clean origin/main checkout (1cbb6aaf), independent of this PR's branch — this is base-branch-red, breaking exact-head-path-policy for every open PR in this repo, not just this one.

Fix ported: opened fix/strix-quick-gate-bootstrap-scope (targeting main directly, not this PR — the fix is unrelated to this PR's docs-only diff) with a corrected awk range that exits as soon as it reaches the next 2-space-indented job header, verified to (a) no longer match on the current opencode-review-target step and (b) still correctly catch a real violation if one were reintroduced. Full suite + this quick-gate script re-verified locally; will link the PR here once pushed. Not re-running this check on this head yet since the fix isn't merged — once it lands on main, the merge scheduler will update this PR's branch and this check will pass on the new head.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Fix pushed: #1528. Once that merges to main, the merge scheduler will update this PR's branch and exact-head-path-policy should pass on the new head.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Correction: #1528 (the fix I linked above) was closed by @seonghobae as a duplicate of the pre-existing, more thorough #1506, which already owns this exact fix (opened ~10 hours before I hit the same bug, and additionally catches a related awk | grep -q SIGPIPE-under-pipefail issue my narrower fix didn't). Once #1506 merges to main, this PR's exact-head-path-policy should pass on the next branch update — tracking #1506 instead.


Generated by Claude Code

@seonghobae

Copy link
Copy Markdown
Contributor Author

Contextual-Orchestrator를 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요.

seonghobae added a commit that referenced this pull request Sep 1, 2026
…hed job's own ceiling (#1532)

Owner-authorized admin bypass past opencode-review: this PR's own diff edits .github/workflows/opencode-review.yml, the required-check pipeline it depends on, and the pre-fix budget (90m poll vs 325m worker job) makes it structurally impossible for this exact fix to pass its own check under the old configuration. Chicken-and-egg bypass explicitly authorized by the repo owner on .github#1500/#1503/#1506/#1527/#1529 today. Full verification (2127 passed, 100% coverage, 100% docstrings) in the PR body.
@seonghobae

Copy link
Copy Markdown
Contributor Author

Contextual-Orchestrator와 관계한 것들을 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요. Timeout은 적어도 3시간으로 잡으세요. 120초 같은 건 당황스럽군요. Opencode와 Noema 는 Coderabbitai 및 Devin 수준으로 실제로 리뷰를 하게 하시오. Strix도 보안 리뷰를 꼼꼼하게 하도록 하시오. 특히 보안 리뷰는 전체 코드로 수행하는 것입니다. Contextual-Orchestrator는 실시간으로 빠르면서 능력이 좋은 모델에 요청을 보내어 시간을 당기시오.

Copy link
Copy Markdown
Contributor Author

Closing as semantically absorbed by protected main rather than restacking this 144-commit-behind documentation branch.

I re-read the live protected-main public/operational documentation before closing. ARCHITECTURE.md already names the Hourly contextual-orchestrator repair gate, renders the contextual-orchestrator sidecar: orchestrator/free gateway in the diagram, and states that the worker auto-discovers across the provider-secret registry rather than binding one provider directly. docs/automation/hourly-review-repair.md already says the write-capable worker uses OpenCode routed through the vendored contextual-orchestrator gateway and scopes the five provider secrets to sidecar provisioning. docs/doctoring/hourly-nvidia-nim-autofix.md already carries the dated 2026-08-31 correction, the orchestrator/free provider contract, and the corrected credential/rollback language. Those are exactly the four-document semantic requirements this PR was created to add; later main work has since evolved the same documents further.

Restacking this historical docs commit would therefore replay already-landed prose into newer architecture/operability material and create another full required-workflow fan-out without a remaining documentation gap. No unique current requirement is discarded; closing unmerged is the lower-risk queue-reduction action.

@seonghobae seonghobae closed this Sep 1, 2026
seonghobae added a commit that referenced this pull request Sep 3, 2026
built and CI-green, genuinely unreachable pending the sandbox

Corrected an initial wrong instinct first: naruon's own "noema_agent.py"
looked like a DDD naming collision with the CI review agent's Noema, but
docs/CWL-MASTER-CONTEXT.md explicitly defines Noema as one shared agent
runtime across three consumers (CI review agent, naruon's do-anything
agent, wardnet's AI SOC quarantine sandbox) -- owner-confirmed per
naruon#1527. Checked the master context before concluding two same-named
components were separate, per this session's own established lesson from
an earlier mistake of exactly that kind.

Found: naruon already has a real, tested, CI-green agent + plugin-
registry system (services/noema_agent.py, services/agent_registry.py,
four open PRs #1527/#1486/#1516/#1537, none authored by either active
peer session) matching the platform plan's own documented registry
architecture almost exactly. The confirmed gap: nothing in the API or
frontend layer ever calls it -- the agent, registry, and manifests are
mutually consistent and fully tested but completely unreachable, and have
been since the agent's original commit two months ago.

This connects directly to item 6: the master context has this agent
running inside quarantine-sandbox-runtime, which a peer session is
separately, actively building (still early-stage, confirmed via direct
coordination) -- wiring the agent to a real trigger before that sandbox
exists would ship an unsandboxed, writeback-capable LLM agent against the
documented design, not merely ship an incomplete feature. Not implemented
this tick pending that dependency.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae added a commit that referenced this pull request Sep 3, 2026
…in Review)

commits/{sha}/status only reflects the legacy Status API, not GitHub's
Checks API that every workflow in this org actually posts through.
Re-checked with commits/{sha}/check-runs: metadata-only gate evaluation
fails on all 4 (expected — real code changes, not eligible for that
script's fast path, and already excluded from this org's own failed-check
enumeration). noema-review genuinely failed on #1527/#1537 — pulled the
job log for #1527 and logged a new, concrete review-failure case (a JSON
repair failure) for item 23's aggregation ask. strix was cancelled on
#1486/#1537, consistent with the already-documented repo-wide
concurrency-group starvation, not a new finding.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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