feat(reviewer-eval): port CI notebook-prose extraction into ci_prompt

[igerber]

Summary

• Tutorial-notebook cases (docs/tutorials/*.ipynb) were guarded out of verify-corpus/run because ci_prompt reproduced only the CI diff exclusion, not the <notebook-prose> block — so such cases would have been reviewed with less context than CI. This ports the extraction with full workflow fidelity: tools/notebook_md_extract.py with the same caps (20k/output, 200k/notebook, 800k aggregate measured in bytes like CI's wc -c, enforced pre-extract-then-append with an omitted-notebooks truncation marker), fail-soft per-notebook extraction, the zero-extracted fallback, close-tag sanitization, the untrusted wrapper + out-of-wrapper warning, and the append-after-diff position. Guards lifted in corpus_loader; README limitation note replaced. Closes the local-review TODO row.
• Security posture (caught by local review and locked by a regression test): the default extractor is resolved against the HARNESS repo root — never the case worktree, whose files are case-controlled content (running its copy would execute case code during prompt assembly). A sentinel-based test proves a malicious worktree notebook_md_extract.py is never invoked while prose still extracts via the trusted copy. Documented divergence mirrors pr_review.md sourcing (CI stages the extractor from BASE_SHA for the same reason).
• Six new adapter tests: wrapper + embedded-close-tag sanitization, zero-extracted fallback, aggregate truncation (chunked), byte-vs-char cap parity (non-ASCII prose), trusted-extractor sentinel, and end-to-end prompt assembly order. verify-corpus passes (2/2), full eval suites green (76).

Methodology references (required if estimator / math changes)

• Method name(s): N/A — reviewer-eval harness only (no library code)
• Paper / source link(s): N/A
• Any intentional deviations from the source (and why): extractor sourced from the current repo instead of each case's base SHA (documented; same rationale as the existing pr_review.md divergence).

Validation

• Tests added/updated: tests/test_evals_adapters.py (6 new; file 16 pass), tests/test_evals_runtime.py unchanged (60 pass); run_eval.py verify-corpus 2/2.
• Backtest / simulation / notebook evidence (if applicable): N/A

Security / privacy

• Confirm no secrets/PII in this PR: Yes

---

Generated with Claude Code

[github-actions]

Overall Assessment

⚠️ Needs changes: one unmitigated P1 prompt-fidelity issue.

Executive Summary

• No estimator, math, weighting, variance/SE, identification, or default-behavior changes were introduced; methodology registry review is not implicated.
• The notebook extraction implementation otherwise tracks the CI steady-state path closely: trusted extractor path, byte-count aggregate cap, fail-soft extraction, sanitization, and append order.
• P1: the robust --name-only -z notebook discovery is gated behind fragile parsing of non--z git diff --name-status, so Git-quoted tutorial notebook paths can silently miss the <notebook-prose> block.
• Minor documentation inconsistencies remain.

Methodology

No findings. Affected methods: none. This PR changes the reviewer-eval harness, docs, TODO, changelog, and tests only.

Code Quality

• Severity: P1
  Impact: build_ci_prompt() only calls build_notebook_prose_block() when touches_notebook(name_status) returns true. But touches_notebook() parses plain git diff --name-status output with tab splitting and startswith/endswith; Git’s default quoted output can C-quote paths with non-ASCII, tabs, quotes, etc. In that case the unified diff still excludes docs/tutorials/*.ipynb, but the prose block is never built, so the local review prompt has neither notebook JSON nor extracted prose while CI’s --name-only -z path would handle it. See tools/reviewer-eval/adapters/ci_prompt.py:L78-L90, tools/reviewer-eval/adapters/ci_prompt.py:L200-L211, and tools/reviewer-eval/adapters/ci_prompt.py:L333-L342.
  Concrete fix: remove the touches_notebook(name_status) gate and always call build_notebook_prose_block() after computing the diff, since it already returns "" when no tutorial notebook changed. Alternatively gate on git_changed_tutorial_notebooks() directly. Add a regression test for a tutorial notebook filename that Git quotes, such as a non-ASCII filename.

Performance

No findings. The added extraction path is bounded by the same per-output, per-notebook, and aggregate caps as CI.

Maintainability

• Severity: P3
  Impact: The build_ci_prompt() docstring says the extractor “falls back to the worktree’s own copy,” but the implementation intentionally defaults to the harness repo copy and explicitly avoids the case worktree. This could confuse future security-sensitive edits. See tools/reviewer-eval/adapters/ci_prompt.py:L329-L341.
  Concrete fix: update the docstring to say the default is the current harness repo extractor, with caller override only for tests or explicit injection.

Tech Debt

No findings. The removed TODO row matches the implemented feature and no new untracked deferred work is introduced.

Security

No direct security finding. The default extractor is resolved from the harness repo, not the case worktree. The adversarial filename prompt-omission issue is covered as the P1 Code Quality finding.

Documentation/Tests

• Severity: P3
  Impact: CHANGELOG.md says the feature is covered by four new adapter tests, but the diff adds six notebook-prose tests, including the trusted-extractor sentinel and byte-vs-char cap parity checks. See CHANGELOG.md:L60-L62.
  Concrete fix: update the changelog coverage sentence to match the actual tests.

Path to Approval

1. Replace the touches_notebook(name_status) gate in build_ci_prompt() with build_notebook_prose_block() unconditionally, or with a gate based on git_changed_tutorial_notebooks().
2. Add a regression test where a changed docs/tutorials/*.ipynb path is Git-quoted in --name-status output, and assert the prompt still includes <notebook-prose>.
3. Fix the extractor default docstring mismatch.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 6c6c500aea295dae09aa3c9eb6589eab10c145b6

---

Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings remain.

Executive Summary

• Prior P1 prompt-fidelity issue is resolved: build_ci_prompt() now always delegates notebook discovery to the null-delimited prose builder.
• No estimator, math, weighting, SE/variance, identification, or default library behavior changes are introduced.
• Security-sensitive extractor sourcing is handled correctly: default path uses the harness repo copy, not case-controlled worktree code.
• Remaining issues are P3 documentation/maintenance nits only.

Methodology

No findings. Affected methods: none. This PR changes reviewer-eval prompt assembly, corpus validation, tests, TODO, README, and changelog only; no Methodology Registry cross-check is required beyond confirming no estimator surface changed.

Code Quality

No P0/P1 findings. The prior quoted-path gate issue is addressed by unconditional build_notebook_prose_block() invocation and a regression test for Git-quoted non-ASCII notebook paths (tools/reviewer-eval/adapters/ci_prompt.py:L340-L347, tests/test_evals_adapters.py:L421-L438).

Performance

No findings. Notebook extraction remains bounded by per-output, per-notebook, and aggregate caps (tools/reviewer-eval/adapters/ci_prompt.py:L66-L70, L246-L284).

Maintainability

• Severity: P3
  Impact: The build_ci_prompt() docstring still says the extractor “falls back to the worktree's own copy,” but the implementation intentionally defaults to the trusted harness repo copy. This is confusing around a security-sensitive behavior.
  Concrete fix: Update tools/reviewer-eval/adapters/ci_prompt.py:L329-L331 to say the default is the current harness repo extractor, with caller override only for explicit injection/tests.

Tech Debt

• Severity: P3
  Impact: The newly moved TODO rows lose important caveats from the prior parked notes. In particular, the stratified multiplier-bootstrap row claims draw blocks reproduce the stream bit-identically, which conflicts with the documented stratum-major RNG concern; the SyntheticControl snapshot row also drops the freeze-contract nuance. This is documentation-only in this PR, but it can mislead future implementation work.
  Concrete fix: Restore or summarize the parked caveats in TODO.md:L52-L56, especially the RNG stream-layout/golden-recapture caveat for stratified survey-PSU tiling.

Security

No findings. The default extractor path is resolved from _HARNESS_REPO_ROOT, not the case worktree, and the sentinel regression covers the malicious worktree extractor case (tools/reviewer-eval/adapters/ci_prompt.py:L335-L339, tests/test_evals_adapters.py:L371-L398).

Documentation/Tests

• Severity: P3
  Impact: CHANGELOG.md says the feature is covered by four new adapter tests, but the diff adds more notebook-prose tests, including trusted-extractor, byte-cap, and Git-quoted filename regressions.
  Concrete fix: Update CHANGELOG.md:L60-L62 to match the actual test coverage.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: d9947597d8e436bb95296d37e620e076423285d6

---

Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

• No estimator, math, weighting, variance/SE, identification, or default library behavior changes are introduced.
• Prior P1 prompt-fidelity concern is resolved: notebook prose discovery now uses the null-delimited Git path via build_notebook_prose_block().
• Security-sensitive extractor sourcing is correct: default extraction uses the harness repo copy, not case-controlled worktree code.
• Remaining issues are P3 documentation/maintenance nits only.

Methodology

No findings. Affected methods: none. This PR changes reviewer-eval prompt assembly, corpus validation, tests, README/TODO/CHANGELOG only; no Methodology Registry or cited-paper mismatch is implicated.

Code Quality

No P0/P1 findings. The changed prompt path handles empty notebook extraction, aggregate truncation, byte-count parity, and quoted Git filenames in the new tests.

Performance

No findings. Notebook prose remains bounded by per-output, per-notebook, and aggregate caps in tools/reviewer-eval/adapters/ci_prompt.py:L66-L70.

Maintainability

• Severity: P3
  Impact: build_ci_prompt() still says the extractor “falls back to the worktree's own copy,” but the implementation intentionally resolves the default extractor from the trusted harness repo. This is confusing around a security-sensitive behavior. See tools/reviewer-eval/adapters/ci_prompt.py:L329-L339.
  Concrete fix: Update the docstring to say the default is the current harness repo extractor, with caller override only for explicit injection/tests.

Tech Debt

No findings. Removing the completed local-review TODO row is consistent with the implemented support.

Security

No findings. The default extractor path is resolved from _HARNESS_REPO_ROOT, not the case worktree, and the sentinel regression covers the malicious worktree extractor case at tests/test_evals_adapters.py:L371-L398.

Documentation/Tests

• Severity: P3
  Impact: CHANGELOG.md says the feature is covered by four new adapter tests, but the changed tests also include trusted-extractor, byte-vs-char cap, and Git-quoted filename regressions. See CHANGELOG.md:L65-L75 and tests/test_evals_adapters.py:L371-L438.
  Concrete fix: Update the changelog test-coverage sentence to match the actual notebook-prose adapter tests.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 25f9a08af573bad60311b86130b085b32870fe8a

---

Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

• No estimator, math, weighting, SE/variance, identification, or default causal-method behavior is changed.
• Prior prompt-fidelity concern appears addressed: tutorial notebook prose is now appended after the unified diff and discovered with git diff --name-only -z.
• The default extractor path is security-conscious: it resolves to the harness repo, not case-controlled worktree content.
• Remaining issues are P3 documentation/maintenance nits only.

Methodology

No findings. Affected methods: none. This PR changes reviewer-eval prompt assembly, corpus validation, docs, TODO, and adapter tests only; docs/methodology/REGISTRY.md estimator contracts are not implicated.

Code Quality

No P0/P1 findings. The new prompt path handles zero extracted notebooks, close-tag sanitization, byte-count aggregate caps, quoted Git filenames, and trusted extractor sourcing.

Performance

No findings. Notebook prose remains bounded by per-output, per-notebook, and aggregate caps in tools/reviewer-eval/adapters/ci_prompt.py:L66-L70.

Maintainability

• Severity: P3
  Impact: build_ci_prompt() still documents that the extractor “falls back to the worktree's own copy,” but the implementation intentionally resolves the default from _HARNESS_REPO_ROOT. This is confusing around a security-sensitive behavior. See tools/reviewer-eval/adapters/ci_prompt.py:L329-L339.
  Concrete fix: Update the docstring to say the default extractor is the current harness repo copy, with extractor_path only for explicit injection/tests.

Tech Debt

No findings. Removing the completed TODO row is consistent with the implemented support.

Security

No findings. The default extractor uses the trusted harness-root path and the regression test covers a malicious worktree extractor at tests/test_evals_adapters.py:L371-L398.

Documentation/Tests

• Severity: P3
  Impact: CHANGELOG.md says the feature is covered by four new adapter tests, but the PR now includes additional trusted-extractor, byte-cap, and Git-quoted-filename regressions. See CHANGELOG.md:L83-L85 and tests/test_evals_adapters.py:L371-L438.
  Concrete fix: Update the changelog test-coverage sentence to match the actual adapter tests.