Skip to content

fix(noema): require observed defect-class probes - #1589

Draft
seonghobae wants to merge 100 commits into
mainfrom
fix/noema-observed-defect-probe-taxonomy-20260901
Draft

seonghobae wants to merge 100 commits into
mainfrom
fix/noema-observed-defect-probe-taxonomy-20260901

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Root cause

The central Noema formal-review gate already required exact changed-line evidence and concrete adversarial probes, but its diversity contract was only structural: two generic probes with different prose could satisfy a material-change verdict without proving that the reviewer attacked independently observed high-value defect shapes. That leaves a false-negative gap around mutable aliases, validation/use races, execution identity, weak test oracles, cross-contract contradictions, authority boundaries, omitted dependency context, and state-machine races.

Observed corpus

The initial executable corpus is grounded in independent reviewer findings from ContextualWisdomLab/noema#528: caller-owned mutable checkpoint aliases, changing-getter/Proxy TOCTOU, cross-execution lifecycle identity, a substring oracle that matched released inside unreleased, and cross-document contract contradictions. These are defect-shape regressions only; this PR does not claim Noema missed the identical historical review or claim parity/superiority over CodeRabbitAI, Devin, or another proprietary reviewer.

Repair

  • add a closed probe_kind taxonomy for mutable_alias, time_of_check_time_of_use, execution_identity, coercion_boundary, test_oracle, cross_contract, authority_boundary, dependency_context, and state_machine_race;
  • fail closed on missing or unknown probe classes;
  • require material source/test verdicts to cover at least two distinct classes, not merely two differently worded probes;
  • explicitly instruct the Noema reviewer to attack these failure shapes;
  • render each class in published review evidence for auditability;
  • add focused regressions and doctoring/quality-gate traceability.

Exact-base companion correction

Full-suite GREEN verification exposed two stale generic tests left after #1587 separated global discovery from orchestrator/free admission. Those tests still used OpenAI as if OPENAI_API_KEY were an eligible free-pool source. Production policy correctly excludes that source while retaining global discovery, so the generic account-cap/limit fixtures now use OpenRouter. No production routing policy is changed here.

TDD and verification

  • hosted RED: run 33499442683, job 99829105882;
  • hosted implementation GREEN: run 33500648307, job 99832942094;
  • focused Noema suite: 118 passed;
  • exact-base stale free-pool fixtures: 2 passed;
  • full repository suite: 2278 passed, 1 skipped, 21 subtests passed;
  • interrogate --fail-under=100 scripts/ci/noema_review_gate.py: 100%;
  • compileall and git diff --check: passed.

The one-shot writer workflow and both temporary transform scripts self-removed after the successful implementation commit and are absent from the final branch. This PR adds a normal least-privilege, SHA-pinned focused quality workflow; the exact current PR head must regenerate ordinary protected-branch evidence before merge.

No review/security/coverage/merge gate is weakened, no provider credential or secret value is changed, and no predecessor-head result is treated as current-head approval.


Devin Review

Summary by CodeRabbit

  • 새 기능

    • PR 리뷰가 정확한 코드 변경과 검증 가능한 증거를 기반으로 수행됩니다.
    • 변경 라인에 연결된 적대적 검증 프로브와 결함 분류를 요구합니다.
    • 증거가 누락되거나 오래된 경우 리뷰 결과를 안전하게 거부합니다.
    • 리뷰 결과를 승인, 변경 요청 또는 댓글로 자동 게시합니다.
  • 문서

    • 결함 분류 체계와 증거 검증 기준을 추가했습니다.
    • 변경 사항을 릴리스 기록에 반영했습니다.
  • 테스트 및 품질

    • 리뷰 증거, 코드 범위, 보안 경계 및 CI 계약에 대한 자동 검증을 강화했습니다.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

Next included review available in 13 minutes.

Check out review usage here.

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: Team

Run ID: ba5c51d7-13e2-4631-b97a-88b274b66e08

📥 Commits

Reviewing files that changed from the base of the PR and between c86d78a and f831194.

📒 Files selected for processing (1)
  • tests/test_noema_observed_probe_doctoring.py
📝 Walkthrough

Walkthrough

Noema 리뷰 게이트와 exact-head CodeGraph 증거 경로를 추가했습니다. 구조화된 결함 프로브와 source-bound witness를 검증하고, 신뢰된 증거가 없거나 오래되면 리뷰 게시를 중단합니다. 관련 회귀 테스트와 집중 CI workflow도 추가했습니다.

Changes

Noema 리뷰 품질 게이트

Layer / File(s) Summary
리뷰 코어와 증거 검증
scripts/ci/_noema_review_core.py, scripts/ci/noema_review_gate.py
PR 메타데이터, diff 위치, LLM 응답, verdict, 리뷰 제출을 제한된 형식과 fail-closed 규칙으로 처리합니다. COMMENT에도 변경 라인과 adversarial evidence 검증을 적용합니다.
Exact-head CodeGraph 증거 materialization
scripts/ci/load_contextual_orchestrator_token.sh, scripts/ci/noema_codegraph_context.sh
PR head와 base를 검증하고 bounded merge base를 계산합니다. 자격증명을 제거한 뒤 고정된 CodeGraph 패키지로 변경 범위 증거를 생성합니다.
검증 계약 회귀 테스트
tests/test_noema_codegraph_context_contract.py, tests/test_noema_comment_evidence_contract.py, tests/test_noema_observed_defect_probe_taxonomy.py, tests/test_noema_probe_source_binding.py, tests/test_noema_observed_probe_doctoring.py, tests/test_noema_review_gate.py
CodeGraph head binding, 증거 누락, taxonomy, source-bound witness, 좌표 형식, COMMENT 검증과 기존 verdict fixture를 검증합니다.
포커스 CI와 제품 문서
.github/workflows/noema-observed-probe-quality-ci.yml, docs/doctoring/noema-observed-defect-probe-taxonomy.md, docs/product-technical-gap-baseline.md, CHANGELOG.md
관찰 프로브 계약, 셸·Python 문법, 커버리지와 diff 형식을 검사합니다. probe taxonomy와 CodeGraph 증거 계약을 문서화합니다.

Estimated code review effort: 5 (Critical) | ~120 minutes

Merge Risk: 🔵 Low · up to c86d7

The PR strengthens review validation and exact-head source analysis, but several newly added contract tests have localized reliability risks: one depends on the working directory, one may accept paths from the wrong workflow trigger, and one duplicates validator taxonomy data. The change is mergeable with explicit owner awareness and follow-up on these test safeguards.

Sequence Diagram(s)

sequenceDiagram
  participant GitHub_Actions
  participant Token_Loader
  participant CodeGraph_Helper
  participant GitHub
  participant Noema_Review_Gate
  participant LLM
  GitHub_Actions->>Token_Loader: 리뷰 환경 초기화
  Token_Loader->>GitHub: PR head와 base 조회
  Token_Loader->>CodeGraph_Helper: 검증된 PR 식별자 전달
  CodeGraph_Helper->>GitHub: exact head와 base fetch
  CodeGraph_Helper->>CodeGraph_Helper: 자격증명 제거 및 CodeGraph 실행
  CodeGraph_Helper-->>Token_Loader: head-bound evidence packet 생성
  Token_Loader->>Noema_Review_Gate: 토큰과 증거 경로 제공
  Noema_Review_Gate->>LLM: 제한된 리뷰 컨텍스트 전달
  LLM-->>Noema_Review_Gate: 구조화된 verdict 반환
  Noema_Review_Gate->>GitHub: 검증된 리뷰 게시
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 Noema 리뷰 게이트에 관찰된 결함 클래스 프로브를 요구하는 핵심 변경을 정확하고 간결하게 설명합니다.
Docstring Coverage ✅ Passed Docstring coverage is 85.11% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 94 functions across 10 files. (4 skipped: 4…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 85.11% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 94 functions across 10 files. (4 skipped: 4 unsupported.)

✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch fix/noema-observed-defect-probe-taxonomy-20260901
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/noema-observed-defect-probe-taxonomy-20260901

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[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

Merge-conflict triage: core infrastructure this PR needs may already be partially shipped, differently, on main

mergeable_state is dirty. git fetch origin main && git merge --no-edit in a scratch clone: conflicts span CHANGELOG.md, docs/product-technical-gap-baseline.md, scripts/ci/load_contextual_orchestrator_token.sh (1 block each — likely ordinary append/drift, not inspected in depth given the finding below) and scripts/ci/noema_review_gate.py (4 blocks, one spanning lines 617-1718 of the conflict markers, over a thousand lines).

Key finding before going further: origin/main already has _noema_verdict_response_format() (line 187) and _required_probe_count() (line 548) — a structured-output schema that requires a count of adversarial probes per verdict. That is exactly the "structural, not diverse" mechanism this PR's own description says is insufficient ("two generic probes with different prose could satisfy a material-change verdict... false-negative gap around mutable aliases, TOCTOU, execution identity..."), which this PR proposes to replace with a closed probe_kind taxonomy (9 classes) requiring distinct classes, not just a count.

So this isn't simply "main moved on, discard this branch's stale copy" (the pattern from most of today's other conflicts) — main has a different, already-shipped, count-based version of the same idea, and this branch's taxonomy-based version is a real, apparently-still-needed improvement over it, but the two are different data models for the same schema field (adversarial_validation.probes: count-only vs. class-tagged) sitting inside the same LLM response_format/verdict-validation pipeline this session already found #1606 also touches and diverges on. Composing them correctly — does the taxonomy layer on top of, replace, or need to be re-derived against main's count-based version? — is a design decision on the required Noema review gate's core validation contract, not something to resolve by picking one side's 1000+ line hunk over the other's.

Per this repo's own standard for this exact situation (#1279, #1280, #1382, and today's #1198, #939, #1009, #1158, #1674, #1606): aborting cleanly, not pushing a guessed resolution. Given #1606 conflicts in the same function on the same file, whoever reconciles noema_review_gate.py likely needs to sequence #1198, #1606, and this PR together rather than merge each independently. No changes pushed to this branch.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4


Generated by Claude Code

@seonghobae

Copy link
Copy Markdown
Contributor Author

Working through this repository's CONFLICTING pull requests, I stopped on this one without pushing anything. Same verdict as #1606, for a structural reason rather than a textual one: main and this branch reorganized the same module in incompatible directions, so merging is a port, not a resolution.

Measured

merge-base                       6eb93bce  (2026-09-01)

scripts/ci/noema_review_gate.py
  on main                        1828 lines   (still one module)
  on this branch                   67 lines   (thin facade)
  new here                       1503 lines   scripts/ci/_noema_review_core.py

delta since merge-base
  main                           +615 / -145   in noema_review_gate.py
  this branch                  +1539 / -1327   (1363 lines moved out to the core)

Git reports four conflict blocks in that file, two of them 521 and 1205 lines, because this branch deleted the body while main kept editing it.

Why there is no side to take

The 615 lines main added since the merge base are not incidental — they are the current review contract:

_noema_verdict_json_schema / _noema_verdict_response_format   strict structured outputs
NoemaModelOutputError / NoemaTransportError                   error taxonomy
NOEMA_REVIEW_FOOTER_MARKER / …CLOSING_MARKER_RE / …BODY_HEAD_RE
MAX_ALLOWED_LOCATIONS_JSON_BYTES / MAX_HTTP_ERROR_BODY_BYTES
_stable_failure_diagnostic, active_phase / served_model telemetry

Taking this branch's side drops all of it. Taking main's side discards the extraction this PR exists to perform. Neither is a merge.

The correct merge is to apply main's +615 into _noema_review_core.py and re-export what the facade must expose. That is mechanical only in outline: the core is a reorganized copy, so main's hunks do not land by offset, and several of those additions are load-bearing in a required review gate — a wrong port changes verdict admission or failure classification silently. Whoever owns the extraction should place them, not a merge pass.

Acceptance oracle

This PR adds five test files; they are the acceptance criterion for any port, and main's suite cannot see them:

tests/test_noema_codegraph_context_contract.py
tests/test_noema_comment_evidence_contract.py
tests/test_noema_observed_defect_probe_taxonomy.py
tests/test_noema_observed_probe_doctoring.py
tests/test_noema_probe_source_binding.py

Run those plus tests/test_noema_review_gate.py after porting — the second is where main's new contract is exercised.

Also worth knowing before the port

main has moved again since this branch's base, in ways that touch exactly these lines:

  • #1681 (merged) added confidence to _NOEMA_FINDING_SCHEMA's properties and required, and a matching or finding.get("confidence") not in {"high","medium","low"} in the findings guard. Both live inside the region this PR extracts.
  • #1606 is blocked on the same class of divergence in the same file — its truncation-recovery retry loop has no counterpart on current main. Whoever does this port should look at #1606 first; landing this extraction may make that one tractable, or may collide with it.

The other three conflicts here (CHANGELOG.md, docs/product-technical-gap-baseline.md, scripts/ci/load_contextual_orchestrator_token.sh — the branch's _contextual_orchestrator_materialize_noema_codegraph versus main's _contextual_orchestrator_install_strix_timeout_compat, both additive shell functions) are ordinary keep-both/keep-both/keep-both work and are not the blocker.

Nothing was pushed — the branch is exactly as you left it.

🤖 Generated with Claude Code

@seonghobae seonghobae added bug Something isn't working priority: medium Normal-priority or P2 work type: bug Defect or incorrect behavior labels Sep 7, 2026 — with ChatGPT Codex Connector

Copy link
Copy Markdown
Contributor Author

2026-09-20 protected-base admission correction for f83119403dc65f0f2cc8cbf10beb3fe75b39f59c.

GitHub reports this Ready PR mechanically non-mergeable. Exact comparison with protected main is diverged and 778 commits behind; no workflow receipts or qualifying independent approval exist. It is therefore not eligible for ordinary integration at this head/base.

Moving it to Draft / Proposed preserves every commit, valid delta, review, and thread. This is not closure or abandonment. Reconcile protected main through an ordinary non-force merge, repair valid findings, run terminal exact-head protection, and obtain qualifying independent current-head approval before returning to Ready. No review dismissal, synthetic status, manual rerun, bypass, Force Push, merge, or Close is authorized.

@seonghobae
seonghobae marked this pull request as draft September 19, 2026 17:56

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: medium Normal-priority or P2 work type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant