fix(coverage): unblock org-wide OpenCode approval + docs(gaps) corrections - #1438
fix(coverage): unblock org-wide OpenCode approval + docs(gaps) corrections#1438seonghobae wants to merge 50 commits into
Conversation
…ma increment #1297 was already merged and #1345/#1326 were closed unmerged, but all three were still listed as pending candidates for this loop's next pass. Replace with current state and record this pass's actual increment: ContextualWisdomLab/naruon#1486 adds a check_calendar_conflict tool to naruon's noema-general-agent, reusing the existing deterministic conflict policy instead of a second one, and clarifies that naruon's Noema and this repo's central review-bot Noema are separate agents sharing only a name.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughSidecar의 stderr 진단 출력 한도를 20줄에서 60줄로 확장했습니다. Provider-family별 catalog 후보 상한을 4에서 8로 변경했습니다. 선행 0이 포함된 잘못된 재시도 설정을 거부하고 관련 테스트와 운영 문서를 갱신했습니다. ChangesSidecar 진단 및 운영 기록
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🔵 Low · up to The change mainly improves failure diagnostics and documentation, but the sidecar still has a configuration edge case where leading-zero zero values can disable retries through an immediate failure; the PR is mergeable with explicit owner awareness and follow-up. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.) Full details: Title checkExplanation 제목의 ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
|
Generated by Claude Code |
naruon#1486 confirms the current sidecar pin (30c6d716...) actually reaches a hosted PR-target run, but still fails closed with a new signature: a non-fatal 413 on ZDR-catalog prefetch (falls back to a live feed) followed by bytez discovery returning HTTP 500, which empties the orchestrator/free pool the same way the already-tracked structural gap describes. Also confirms opencode-review's gate correctly fails closed awaiting the async dispatch verdict on fresh heads (naruon#1486, .github#1438) — expected, not a defect.
|
Generated by Claude Code |
…tail The review sidecar's live warm-up preflight already records a bounded error_type/http_status per rejected route, but only into a JSON artifact -- not the CI job's visible console log. That made a real fail-closed incident impossible to diagnose as transient or not without downloading the artifact separately, and led directly to a misdiagnosis this pass corrects in docs/product-technical-gap-baseline.md. Add _log_preflight_rejections (mirrors the existing _log_discovery_errors visibility fix), a matching sanitizer allowlist entry, and widen the failure-path stderr tail from a fixed 20 lines to a named SIDECAR_STDERR_TAIL_LINES=60 so discovery errors plus preflight rejections plus summary lines can no longer silently truncate. Companion to ContextualWisdomLab/contextual-orchestrator#923, which fixes the analogous single-shot discovery fetch. This repo's own completion-warm-up-probe call site is deliberately left single-shot; see that PR's description for the latency/amplification risk that ruled out retrying it blind.
Adversarial re-investigation (triggered by direct feedback that a single provider erroring should never fail-close org-wide review CI) found the "2026-08-30 post-#1486/#1438 wake" entry misdiagnosed the incident on two counts: Bytez can never populate orchestrator/free regardless of HTTP status (_parse_bytez never sets is_free), and the 413 line is the sidecar's own unconditional self-test, not a live ZDR-prefetch fallback -- also present in two earlier entries, flagged here rather than hand-edited there. The actual terminating message was "review sidecar preflight failed" (a live warm-up-probe rejection), not the "no eligible models" path those entries claimed. Records the real root cause (two single-shot HTTP call sites with no retry) and the fix that follows: contextual-orchestrator#923 (discovery retry) and this repo's own #1438 (preflight-rejection visibility + wider stderr tail). Updates §5.1 to track both to merge.
seonghobae
changed the title
main advanced with the owner's own parallel investigation into the same sidecar-preflight incident this branch was fixing, with far more precise evidence than this branch's own analysis had (actual hosted-run preflight/discovery artifacts, not just log-pattern reading). The real root cause turned out to be contextual_orchestrator_review_policy.py's family_cap selecting the same alphabetically-first candidates every run -- 2 of which are permanently-retired NVIDIA model ids returning HTTP 404 forever, not a transient failure -- plus a too-tight gateway smoke-test timeout and a max_tokens/probe-budget desync. All three are already fixed on main (family_cap 4->8, gateway timeout 30s->120s, max_tokens 16->4096). Conflict resolution: - CHANGELOG.md / docs/product-technical-gap-baseline.md: kept both sides' entries; revised this branch's own "correction" entry to defer to main's much more thorough "sidecar-preflight outage: consolidated evidence" entry as the authoritative root cause and fix, rather than presenting this branch's own generic "two call sites, no retry" theory as confirmed. That mechanism is still real (kept, reframed as independent/complementary), just not what caused this specific incident. - scripts/ci/contextual_orchestrator_review_sidecar.sh: kept main's new preflight-report console dump and this branch's SIDECAR_STDERR_TAIL_LINES threading -- complementary, not overlapping. Deliberate follow-up simplification in the same commit: dropped this branch's own _log_preflight_rejections (launcher.py), its sanitizer allowlist entry, and its tests -- main's own `log "sidecar preflight route evidence: ..."` fix already surfaces the same already-bounded-safe preflight_report JSON to the console, achieving the same operator-visibility goal more directly. Keeping both would have been duplicate code solving the same problem twice. Full suite: 1897 passed, 1 skipped, 21 subtests; coverage 100% on all touched files (pre-existing pingora_edge_policy.py:274 gap, owned by #1398, unaffected); interrogate 100%; bash -n clean.
seonghobae
changed the title
…ixes Summarizes the wakeup that found main had advanced with the owner's own deeper investigation into the same bytez/preflight incident, reconciled this branch's three open PRs against it (merged current main/develop into each, dropped the now-redundant preflight-visibility fix, marked all three ready for review), and explicitly defers .github#1347's merge conflict to a dedicated next pass given it overlaps a file main has independently hardened for SSRF.
… finding naruon#1486 and contextual-orchestrator#923 both hit a PR-governance metadata-gate block that looked like a blocking CodeRabbit finding but was actually CodeRabbit's own "approval pending, not reviewed yet" state -- every ContextualWisdomLab repo is below CodeRabbit's 10-GitHub-star automatic-review threshold, so it never reviews a new commit without an explicit @coderabbitai review trigger. Triggered both PRs manually; documents the structural gap and a deferred central-automation candidate fix.
|
@coderabbitai review Generated by Claude Code |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin review on #1438 flagged two real issues in the CodeRabbit gap-baseline entry: bare naruon#1486/contextual-orchestrator#923 references don't create durable cross-repo links (missing the org prefix), and the claim that "every org PR" is affected outran the evidence (only 4 repos were actually checked). Both fixed.
|
_Generated by Claude Code Generated by Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… a fix CodeRabbit correctly flagged that "already fixed"/"resolves" overstates the 4->8 family-cap raise: it reduces the odds of the same retired/timed-out candidates being selected every run, but doesn't guarantee against it, and hosted-run confirmation of the fix is still pending. Softened both the changelog bullet and the gap-baseline correction entry to "mitigates ... hosted confirmation remains pending".
|
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
CHANGELOG.md— repository behaviordocs/product-technical-gap-baseline.md— operator or user guidancescripts/ci/contextual_orchestrator_review_sidecar.sh— review and security gate shell pathtests/test_contextual_orchestrator_review_sidecar_contract.py— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: CHANGELOG.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: CHANGELOG.md"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: product-technical-gap-baseline.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: product-technical-gap-baseline.md"]
R2 --> V2["docs review"]
Evidence --> S3["CI script: contextual_orchestrator_review_sidecar.sh"]
S3 --> I3["review and security gate shell path"]
I3 --> R3["Review risk: CI script: contextual_orchestrator_review_sidecar.sh"]
R3 --> V3["bash -n plus Strix self-test"]
Evidence --> S4["Test: test_contextual_orchestrator_review_sidecar_contract.py"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test: test_contextual_orchestrator_review_sidecar_contract.py"]
R4 --> V4["targeted test run"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
c11b68c2e3cb4d14099c0925f4239c1ae13dc674 - Workflow run: 33310753001
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: CHANGELOG.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: CHANGELOG.md"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: product-technical-gap-baseline.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: product-technical-gap-baseline.md"]
R2 --> V2["docs review"]
Evidence --> S3["CI script: contextual_orchestrator_review_sidecar.sh"]
S3 --> I3["review and security gate shell path"]
I3 --> R3["Review risk: CI script: contextual_orchestrator_review_sidecar.sh"]
R3 --> V3["bash -n plus Strix self-test"]
Evidence --> S4["Test: test_contextual_orchestrator_review_sidecar_contract.py"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test: test_contextual_orchestrator_review_sidecar_contract.py"]
R4 --> V4["targeted test run"]
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
_load_changed_files's post-loop PolicyError at the end of the pagination loop can never execute: 31 full 100-item pages would push the file count past 3,000 during page 31's own iteration (30 full pages = exactly 3,000), tripping the len(files) > 3_000 raise inside the loop before the outer range(1, 32) can ever exhaust without an early return or that inner raise. This dead line has been silently failing this repo's org-wide coverage-evidence gate (fail_under=100 on scripts/ci) for every PR reviewed through the central OpenCode/Noema/Strix dispatcher -- confirmed via live Actions logs on multiple unrelated PRs (.github#1161, #1438) all showing "Coverage failure: total of 99 is less than fail-under=100" at this exact line, which in turn blocks opencode-agent from ever posting an APPROVED verdict anywhere in the org. Marked pragma: no cover with a justification, matching this repo's existing convention for provably-unreachable defensive code (see other pragma: no cover sites in scripts/ci/*.py). Verified: full suite 1897 passed, 1 skipped, 21 subtests; coverage TOTAL 100% (9966/9966 statements, 3926/3926 branches); interrogate 100%.
Consolidates a 5-agent investigation into why opencode-agent had not posted a verdict on any of the three tracked PRs: naruon#1486 is stuck on a stale scheduler thread-count snapshot, contextual-orchestrator#923 is missing a cross-repo dispatch credential in its scheduler run, and .github#1438's dispatch ran but was blocked by the pingora_edge_policy.py coverage bug fixed in the preceding commit. Also records hosted-run confirmation that the earlier family_cap sidecar mitigation is now working (3/3 post-fix runs clean), while flagging a separate, still-open "healthz passes then completion request hangs" signature on the same commit.
seonghobae
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… successful re-run
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…s, overclaimed root cause)
…nce only, not a required check)
…hmentReparseWorker
…idecar-startup hang)
|
The required Noting the escalation: this is the same "preflight passes, then the actual scan-time completions stall" sub-symptom that just hit Generated by Claude Code |
…stall sub-symptom
|
A second The job's own internal bounded-retry loop made 3 attempts (135s, 65s, 73s — each fast, no hang), and all three failed on the identical model: This repeating 3/3 on the exact same model/agent rules out transient flake — it's a deterministic selection/compatibility bug: a vision-only model ( Not re-running this one: since it's deterministic (not flaky), a re-run would very likely hit the same model again and fail identically — that would just burn CI time repeating already-conclusive evidence. Not this PR's diff (the model-selection/family_cap logic lives in Generated by Claude Code |
…nly model in orchestrator/free)
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…pothesis (Devin review)
|
Fresh exact-head sidecar acceptance evidence to absorb on this current writer head The present diff widens only the pre-health stderr tail and validates zero-valued retry limits; it does not expose post-ready process death. Please preserve those compatible fixes and add a deterministic RED where the sidecar passes readiness then exits before gateway preflight. The narrow repair should retain/capture process exit status plus a scrubbed bounded stderr tail on the gateway failure path. This must remain an infrastructure/provider non-pass, not a fabricated review or source finding. |
…hable one in gateway preflight When the gateway-preflight retry loop exhausted every attempt with no usable HTTP response, it always recorded generic "transport exhausted" evidence, even when the sidecar process itself had already exited after passing readiness -- discarding the single most useful diagnostic (the process's own exit status and stderr). Now checks kill -0 "$sidecar_pid" first (safe for a bash-owned background job: a failing kill -0 means bash's own job table has already reaped it, so the following wait retrieves the real cached exit status -- the same pattern the pre-existing healthz branch already used) and records distinct evidence plus a distinct fail message when the sidecar has exited. Reported via a cross-agent coordination comment on .github#1438, verified against issue #1399 and PR #1460 before acting on it. Exact-head evidence: Strix run/job 33341290448/99337282309 for #1460 target 2cc819a. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6UJHYbfbGdHfYPjgbVhAr
|
Fixed and pushed (commit Previously, when every gateway-preflight attempt exhausted with no usable HTTP response, the script always recorded generic Fix: before recording the generic transport-exhausted evidence, check TDD: new structural contract test ( CI is currently running on the new head; will keep watching. Generated by Claude Code |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…s shipped Cross-references naruon#1486 commit 733f22cf, which applied AttachmentReparseWorker's already-fixed dedicated-lease-connection and starvation-free-cursor pattern to NewsdomRecognitionWorker's own attachment and document sweeps -- the deferred candidate this doc recorded when the reparse worker's version of these bugs was fixed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6UJHYbfbGdHfYPjgbVhAr
|
Independent exact-head verification for
The post-readiness process-death boundary is now causally typed: a dead child is reported as This is source/test evidence only. Protected-main integration followed by an operational review canary is still required; cancelled required Strix and absent formal approval remain non-passing, so no merge is attempted. |
…alwisdomlab-commercialization-afow1j # Conflicts: # .gitignore # CHANGELOG.md # docs/product-technical-gap-baseline.md
Fold in this pass's resolution of .github#1438's dirty mergeable_state (three-file conflict against a since-advanced main: .gitignore, CHANGELOG.md, docs/product-technical-gap-baseline.md itself), the ordinary merge commit ca4c5ad, and full-suite/coverage/interrogate re-verification, plus naruon#1486's current-head Check status.
2 participants
Summary
Exact current identity and evidence
18987a7191f070fdcb134d5feb96a07442c39a98main@1ff8268255b061461d9d49b4cab4febf9a8e7bfa.gitignore,CHANGELOG.md,docs/product-technical-gap-baseline.md,scripts/ci/contextual_orchestrator_review_sidecar.sh, and two sidecar tests.scripts/ci/pingora_edge_policy.pyis not in the current diff. The earlier coverage-fix narrative below is historical and must not be treated as this PR's current source delta.50febfe7a9bd74c8c33d1eef6526a33f116f5c2fand cannot satisfy this head.50febfe7, passed discovery/health/gateway preflight, then timed out after 5400 seconds and emittedSTRIX_PROVIDER_UNAVAILABLEwithout an authoritative scan result; attempt 2 was cancelled during provisioning. This is provider/infrastructure evidence recorded on fix(ci): gate Strix's orchestrator/free access on live diversity evidence #1437, not a source finding or current-head pass.Original summary (docs correction + sidecar tail widening — unchanged):
docs/product-technical-gap-baseline.md): the "2026-08-30 post-chore(fuzz): remove dead duplicate fuzz target #1486/fix(coverage): unblock org-wide OpenCode approval + docs(gaps) corrections #1438 wake" entry misdiagnosed the incident on two counts, found by a 7-agent investigation:orchestrator/freeregardless of its HTTP status (_parse_byteznever setsis_free) — its logged HTTP 500 changed nothing about the outcome.request_failed status=413line is the sidecar's own unconditional offline self-test, not a live ZDR-catalog prefetch that "fell back" to anything.main's much more thorough "sidecar-preflight outage: consolidated evidence" entry as the authoritative root cause, rather than presenting this branch's own generic "two call sites, no retry" theory as confirmed for this specific incident (that mechanism is still real, just not what caused this one — seeContextualWisdomLab/contextual-orchestrator#923).SIDECAR_STDERR_TAIL_LINES: widens the sidecar's failure-path stderr tail from a fixed 20 lines to a named constant (60), complementary to (not overlapping with)main's new preflight-evidence log line — so discovery-error diagnostics can't be silently truncated alongside it.Also includes the original §5.1 next-increment-list refresh (#1297 already merged; #1345/#1326 closed unmerged), the naruon-Noema role-clarification entry, and a CodeRabbit 10-star/rate-limit gap entry, all from earlier in this branch's history.
Verification
PYTHONPATH=. python -m coverage run -m pytest tests -q→ 1897 passed, 1 skipped, 21 subtests, coverage TOTAL 100%.python -m interrogate→ 100.0%.bash -n scripts/ci/contextual_orchestrator_review_sidecar.sh→ syntax OK.PYTHONPATH=. python -m pytest tests/test_product_technical_gap_baseline.py -q→ 5 passed (doc contract markers intact).PYTHONPATH=. python -m pytest tests/test_pingora_edge_policy.py -q→ 61 passed.Related
ContextualWisdomLab/contextual-orchestrator#923— companion discovery-side retry fix (independent resilience improvement, not the fix for this specific incident).ContextualWisdomLab/naruon#1486— the PR whose CI run first surfaced this incident.ContextualWisdomLab/.github#1398— the owner's own in-flight, more extensive coverage-evidence/Python-lock work; this PR's dead-code fix is narrowly scoped and orthogonal to it.Summary by CodeRabbit
개선 사항
버그 수정
"00","0000")을 올바르게 거부합니다.문서
테스트