v0.11.0

Added

• Credential redaction in reports and publish path (RENG-113): the security expert's Evidence quoting a config line with a real password used to land in the report file and (with --publish) in the GitLab/GitHub MR discussion verbatim. Three changes close the leak:
  • Evidence blocks are redacted at the renderer by default: every piece of LLM-derived prose the team renderer / per-expert renderer / repo-review renderer emits (Evidence, summary, recommendation, the raw-LLM-response excerpt, the lead-summary TL;DR and conflict resolutions) is passed through crate::models::redact_text, which replaces detected credential values with a marker that keeps the diagnostic context — kind, len and a 16-hex-char SHA-256 fingerprint — so a reviewer still knows which line had the hit. The detector recognises password = "…", api_key = "…", token = "…", private_key = "…", PEM blocks, https://user:pass@host URLs, Authorization: Bearer … headers, and the value-only shapes (glpat-…, ghp_…, AKIA…, sk-…). Benign prose passes through unchanged. The publish-time error message prints the first 8 hex chars of the same fingerprint; an operator who greps the local file for that 8-hex prefix lands on the offending marker.
  • Report files land at 0600: the timestamped copy under output_dir (~/.config/review-engine/reports/review_*.md by default) and the explicit --output file are both chmod 0600'd after writing on Unix, so even a host with a permissive 022 umask does not leave the file readable to other accounts. The override is exercised end-to-end by the report-output tests (write_report_text_lands_the_timestamped_copy_at_0600 / …_the_explicit_output_file_at_0600).
  • --publish pre-scans the body and refuses on a hit: before any provider call, both the board body and every selected inline-note body are scanned with crate::models::scan_for_publish. A hit returns refusing to publish: <surface> body carries N suspected plaintext credential(s); re-redact the offending finding locally before retrying. Detected: kind=…, key=…, len=…, fp=… — names the surface ("board body" or "inline note for file:line"), the credential kind and the value's fingerprint, never the value itself. Defence in depth: the renderer already redacted Evidence, so a real hit means a value slipped through a renderer path we forgot to update, or lives in a field the renderer doesn't cover (Finding.title / Finding.recommendation for inline notes). The gate refuses the whole publish — board and inline notes alike — so a partial post never lands.
• Gated ticket-compliance check surfaces a per-ticket structured verdict alongside the existing review (M40, r1 + r2 + r3): when the operator enables the gate — [compliance] enabled = true in the project-level .code-audit-config.toml (extended via the precedence chain; a deployment that never opens the section stays exactly as it was) or CODE_AUDIT_COMPLIANCE_ENABLED=true on the auto-detect path (the env pass runs AFTER the project-file lift in the auto-detect branch to match the --config path's load_and_apply ordering — env wins in both directions) — the runner parses the MR / PR description for ticket references (GitLab: #N (local, ≤ MAX_SHORTHAND_ISSUE_DIGITS = 6), org/repo#N, <work_item_URL>, <issues_URL>; GitHub: #N, org/repo#N, <issues_URL>; GitLab !N is NOT a ticket form — it is the MR sigil and is intentionally unmatched, matching pr-agent's ticket_pr_compliance_check.py), fetches each ticket body via the provider the MR came from (github_api_base_from_mr_url maps github.com / *.github.com (port-stripped) to https://api.github.com and any other host to {scheme}://{host}/api/v3 for GitHub Enterprise Server — the previous mapping returned the MR host, so a real github.com PR fetched https://github.com/repos/... and 404'd), runs exactly one LLM call that scores the diff against the ticket's requirements (capped at MAX_TICKETS_PER_REVIEW = 3 matching pr-agent), and renders a new ## Ticket compliance check section alongside the per-expert findings. The four-gate contract: enabled = false → 0 extra LLM calls, 0 extra HTTP calls; enabled = true + no refs → 0 extra LLM calls, 0 extra HTTP calls; enabled = true + refs found → exactly 1 LLM call; every fetch failed → 0 LLM calls + a fetch-failure section (notes, never review-blocking). Each successfully-verified ticket carries the 4-bullet **Requirements** (from the ticket) / **Satisfied** / **Not satisfied** / **Needs verification** shape (render_check at src/compliance/render.rs:127) citing file:line evidence in the bullets; a fetched-but-not-analysed bucket (output.unanalysed_tickets, rendered as ### <provider> <source> — fetched, no analysis) is kept separate from the output.fetch_failures bucket (### <provider> <source> — fetch failed, with URL + failure reason), so the reader can tell "the fetch failed" from "the fetch succeeded but the LLM never produced a verdict". Both review entry points invoke the step: the webhook path (run_review_common at src/server/mod.rs) and the REST path (run_review at src/server/api/review/resolve.rs); static-diff sources pass None, None and skip the step. The token is parameterised — no GITLAB_TOKEN / GITHUB_TOKEN env reads left in src/compliance/; both providers accept the token as a function argument threaded from the caller. ComplianceOutput carries #[serde(default)] on fetch_failures / unanalysed_tickets / notes for forward-compat with never-merged pre-M40-r2 stored JSON, and docs/config-schema.md now lists the [compliance] block (with the env override and the precedence table including the section). Pinned by the integration suite under tests/compliance/main.rs: gate_off_means_zero_llm_calls_and_zero_provider_calls / gate_on_with_no_ticket_refs_means_zero_llm_calls_and_zero_provider_calls / gate_on_with_gitlab_ticket_refs_means_exactly_one_llm_call / gate_on_with_github_ticket_refs_means_exactly_one_llm_call (the four-gate contract); cap_drops_excess_tickets_to_max (a 5-ref description is capped at 3 with a capped at 3 note recorded and only the first three fetched); the GitHub wiremock leg asserts 1 provider call + 1 LLM call, the GitLab leg asserts checks[0].parse_error.is_none() in addition to the call count; and plan_decision_matches_integration_assumptions (the plan/preview contract). The URL / API-base unit pins live alongside their implementations: gitlab_bang_sigil_is_ignored / gitlab_work_item_url_extracts_project_and_iid / gitlab_work_item_url_on_other_host_is_dropped at src/compliance/parser.rs:336/354/367 (the !N-as-MR-sigil exclusion M40 r2 parity with pr-agent + the GitLab URL surface), and github_api_base_maps_github_com_to_api_github_com / github_api_base_maps_subdomain_github_dot_com_to_api_github_com / github_api_base_maps_ghe_server_to_api_v3 / github_api_base_strips_optional_port_on_github_com / github_api_base_passes_through_wiremock_hosts at src/compliance/fetch.rs:279/293/302/317/328 (the github.com → api.github.com / GHE → /api/v3 production mapping). The env-ordering pin lives in src/config/resolver/tests.rs:147 (test_apply_env_overrides_compliance_enabled_true — the env-pass move on the --config path), :326 (test_resolve_config_lifts_compliance_from_project_toml — the auto-detect project-file lift), and :356 (test_resolve_config_env_overrides_project_toml_compliance — env-wins-over-file both directions on the auto-detect path). The M43 local install + smoke run on v0.10.52 (reports/local-install-smoke.md) end-to-end produced 1 analysed ticket with zipformer-CTC surfaced under **Needs verification** — the LLM put the unresolved model-integration point in the unresolved bucket rather than satisfied, matching pr-agent's surface; the satisfied bullets cited refine.rs:264-282 / refine.rs:319-346 / config.rs:45-46 / licenses/fire-red-asr2/.

Changed

• Per-chunk code_quality coverage is exposed in the report (M26): every chunk's pre-call is summarised in a new chunk_summary field on RepoReviewOutput (and the Markdown renderer renders a Chunks Reviewed appendix) so a reader can see "X of Y chunks reviewed" without grepping the per-expert fallback: true rows. The summary carries total / reviewed / not_reviewed counts plus the per-failure detail (module + real loc + verbatim reason) so the LOC the model never saw is quantified. chunk_fallback_score's summary now leads with the chunk not reviewed prefix (CHUNK_NOT_REVIEWED_PREFIX), a stronger signal than the fallback: true flag (which the LLM_FALLBACK_SCORE placeholder also carries for unrelated expert failures). Counts are also published at INFO so the logs carry the same number the JSON / Markdown do. The appendix is suppressed on the local-only path (total == 0) so reports without an LLM run don't gain an empty section. Pinned by chunk_summary_total_below_one_suppresses_the_appendix (the empty-summary case), chunk_summary_with_failures_renders_total_and_per_failure_rows (the renderer surface, both Markdown and JSON round-trip), chunk_summary_zero_failures_with_some_reviewed_omits_table_but_keeps_totals (the partial-success case), and chunk_fallback_score_carries_not_reviewed_prefix_and_module (the prefix contract).
• llm_experts accept the legacy review: { … } schema as a fallback (M18-W2, M26): an older prompt template wrapped the LLM payload in a top-level review: mapping (e.g. review: { score: …, findings: […] }) instead of putting score / summary / findings at the root. The modern parsers silently dropped this to a 70-fallback. parse_expert_yaml now promotes the inner mapping when the modern top-level keys are absent and logs at INFO with reason_code = "legacy_review_fallback" so config audits can see the migration surface. The modern form wins when both surfaces are present (the legacy promotion is only a fallback for configs still pinning the old prompt template). Pinned by parse_expert_yaml_promotes_legacy_review_findings_to_top_level (the recovery shape), parse_expert_yaml_legacy_wrapper_without_findings_is_not_promoted (an empty review: {} wrapper is the same shape as no wrapper, the fallback path keeps running), and parse_expert_yaml_modern_keys_take_precedence_over_legacy_wrapper (both surfaces present → modern wins).
• parse_expert_yaml carries a stable reason_code field on every fallback path (M18-W2, M26): the previous WARN-level logs embedded the diagnosis in free-form prose ("LLM response failed YAML parse; …") and consumers had to substring-match to aggregate. The new ParseReasonCode enum pins four stable lowercase identifiers — empty_response, missing_expected_keys, unparseable_yaml, legacy_review_fallback — paired with every warn / info log line, so dashboards can filter on reason_code without re-implementing RENG-35's lesson (a 401 whose body contains "connection" is exactly the class of fragility these codes exist to avoid). Pinned by parse_reason_code_strings_are_stable.
• call_scoring exposes by-expert sample-fallback rate (M18-W2, M26): when scoring.score_samples > 1 the function returns samples_attempted (total calls attempted) and samples_failed (call error / unparseable / schema-conform-but-no-score) on ScoringCall, and both flow into ExpertScore and ExpertScoreOutput. The single-shot path (n = 1) reports samples_attempted = 1, samples_failed = 0 so the rate field is uniform across paths; the public JSON contract omits the rate via skip_serializing_if = "Option::is_none" whenever the gate does not enable multi-shot sampling, so a default-config report does not carry a misleading "0% fallback rate". INFO log carries the by-expert rate inline for parity. Pinned by test_convert_scores_samples_rate_is_none_when_samples_is_none (the convert_scores layer — single-shot input has samples: None → output has both rate fields None; multi-sample input has the rate populated), test_expert_score_output_json_omits_rate_when_samples_is_none (the JSON contract end-to-end), call_scoring_reports_samples_failed_count (4 samples / 2 fail / rate 0.5), call_scoring_reports_zero_failures_on_clean_samples (clean run), and call_scoring_single_shot_records_one_attempt_no_failures_on_success (n=1 path).
• M18-W3 already in place — pinned by hygiene_only_expert_is_excluded_from_risk_surfaces (M26): the report-layer filter (scoring::is_hygiene_finding + expert_is_hygiene_only) was already correct after RENG-115 round 3 — note-only experts without a [hygiene] tag (e.g. test_coverage's "No CI test step detected") stay in risk_categories and top_risks, while code_style's [hygiene]-tagged missing-config reminders do not. M18-W3 is folded in: the audit's "note-only expert 不进 Top Risks / Risk Map" was the r2 surface, the r3 change narrowed the predicate back to tag-only, and the contract is now pinned by an explicit three-shape test (hygiene-only excluded, mixed kept, note-only kept).
• Four mechanical-FP skip rules close RENG-114's 20 false positives (M28, r1 + r2): the M25 self-audit surfaced 20 [mechanical] security findings, all of them false positives — three categories: CHANGELOG.md prose describing the regex itself (×3), :show-password="…" Vue prop bindings whose value is a Vue expression (×3), and Rust docstring / line-comment examples documenting the redaction format (×13). The mechanical sweep also had a 4th FP at src/store/rows.rs:143 where api_key: encrypt_non_empty(…) is a struct field whose value is a function call, not a hardcoded literal. The 20-FP corpus pulled security from a clean 100/100 down to 68/100 on the M25 audit run (4 pts × 8-hit cap = 32 max deduction; the noise still inflates the finding count and undermines the score credibility). Four mechanical-skip layers compose to suppress them, while every real-positive shape keeps reporting: (a) Markdown documents — paths ending in .md / .markdown are skipped wholesale at the entry level; the scan is for source code, and a literal credential embedded in a CHANGELOG / README is a doc-hygiene issue, not a code leak; (b) Rust line comments — every line that, trimmed, starts with // is exempted from regex matching (covers regular //, doc ///, and module-level //!); real assignment forms outside any comment / markdown / Vue / cfg-test shape keep reporting; (c) Vue directive bindings — for .vue files, regex hits whose attribute LHS starts with : (shorthand) or v-bind: (long form) are suppressed; static attributes (show-password="hunter2_real" with no directive prefix) keep reporting; the walk-back finds the full hyphenated LHS (:show-password, not just -password); (d) Identifier-only api.?key matches — gated by BOTH the separator being : AND no opening quote consumed; the matched value is an identifier reference (api_key: encrypt_non_empty(…)), not a hardcoded literal; quoted struct literals (api_key: "secret") keep reporting because the regex consumed an opening quote; api_key = "…" / api_key = someVar keep reporting because the separator is =. All seven layers compose — a production api_key = "secret" line in a regular .rs file outside any #[cfg(test)] block still surfaces, while every reproducer from the M25 self-audit is suppressed. Pinned by scan_security_patterns_in_text_skips_m28_regression_corpus (the 20 reproducer sites → 0 findings), scan_security_patterns_in_text_keeps_real_positives_after_m28_skips (the five real-positive shapes: api_key = "literal" / api_key: "literal" (separator : but quote consumed) / static Vue password="literal" / production let password = "real" in a .rs file / 20+ char token = "literal"), scan_security_patterns_in_text_skips_vue_directive_bindings (M28 r2: directive binding :show-password="fn(…)" / v-bind:password="fn(…)" / :api-key="identifier" all suppress), scan_security_patterns_in_text_keeps_static_vue_attributes (M28 r2: static form show-password="hunter2_real" reports), scan_security_patterns_skips_markdown_entries (entry-level belt-and-braces), and the M19 cfg-test / placeholder pins (revert yields exactly 4 findings on the sirena reproducer, not 1).
• The lead summary stops hiding what was cut — File routing, ## Coverage detail, parse-failure labelling, confidence-filter banner, and ## Injected Context table (M32): five report-layer truthfulness fixes in one release. (i) Coverage rename + dedicated detail section — the lead summary's coverage banner now reads File routing: X/Y files assigned to a successful expert task, and a new ## Coverage detail section carries the hunk-level ratio; the audit's 21-of-21 / 0-of-1211 regression no longer reads as one optimistic Coverage: 100% line. coverage_insufficient still gates unverified; the run-level verdict is CoverageSummary::is_sufficient() at crate::coverage (src/coverage/mod.rs:103), which the consolidator projects to the coverage_insufficient flag the renderer reads; the per-file CoverageStatus enum (Pending vs Touched on each CoverageTarget) is bookkeeping, the summary is the verdict. (ii) AGENTS.md / file_contents truncation explicit — read_local_agents_md_with_meta and build_file_contents_section_with_meta surface original_bytes / injected_bytes / truncated / sha256 fingerprint per source, and the lead summary renders a ## Injected Context table so a reader sees the truncation rather than guessing from the prompt body. (iii) Adjudicate / confidence filter banner — ConsolidatedReport.confidence_downgraded carries the exact count; the TL;DR appends a Confidence filter downgraded banner with min_confidence / drop_low_confidence hint, and a tracing::info! line with reason_code = "confidence_filter_downgraded" so log filtering by reason_code exposes the silent re-routing. The banner wording uses the consolidator-side ConfidenceFilterPolicy field (replacing the prior thread-local that refresh_assessment lost across worker threads; see P2-3 below) and pins the user-runnable remediation — lower min_confidence or lower adjudicate_min_severity, not the inverted "raise min_confidence" advice the r1 banner carried. (iv) Verification pass: ran on N, skipped M, dropped K — the M31 struct is projected onto the M32 simple shape ConsolidatedReport.verification_stats: {checked, skipped, dropped} and rendered at the bottom of the lead summary; the "ran, dropped 0" false-reassurance is replaced by the full triple. The renderer no longer calls every skip a parse failure — the M31 skipped aggregate sums three kinds (LLM errors + parse failures + empty verdict lists); only one is a real parse problem, and the bilingual copy now lists all three causes and points readers at logs.ndjson for the breakdown. (v) parse-failure label — fallback_report / parse_aggregator_response carry 未审 / un-reviewed (NOT no issues found) wording; the lead summary surfaces a ⚠️ parse failures (未审 / un-reviewed) line listing the affected expert names. The orchestrator-side wiring that makes (i)–(v) end-to-end visible in real runs (vs the staged Default::default() placeholders M32 shipped) is owned by M35 — see below.
• LICENSE / CHANGELOG / NOTICE (and variants) no longer enter the expert prompt (M18-W9, M33): diff::filter::should_ignore previously only matched known binary / lock / generated extensions and paths (Cargo.lock, *.png, node_modules/, target/, vendor/, etc.). Legal and changelog documents — LICENSE, LICENSE.md, LICENSE.txt, LICENSE-APACHE / -MIT / -BSD, the British spelling LICENCE, CHANGELOG, CHANGELOG.md, NOTICE, NOTICE.txt, plus per-dependency license files under Cargo's licenses/ and the uppercase LICENSES/ directories, and .spdx SBOM files — were passed to every expert, which wasted 1–3K tokens per large PR and triggered false positives when the security expert regex-matched the literal string Apache-2.0 against its credential patterns (the original RENG-19 / RENG-113 reproducer used exactly such a file). Three new lookups: an IGNORED_FILENAMES constant for the base-name match (compared case-insensitively against the final path component, so LICENSE / license / License / LICENSE-APACHE / LICENCE all match, but src/licensor.rs, src/license_checker.rs, myfilelicense, and LICENSE_HELPER.rs do not — the regression for the woff / ttf class is preserved); licenses/ and LICENSES/ added to IGNORED_PATHS for the per-dependency directories Cargo and monorepos use; .spdx added to IGNORED_EXTENSIONS for SBOM files. The pin tests cover both directions, in the same shape the existing font-extension regression uses (should_ignore_license_filename, should_ignore_license_with_dashed_suffix, should_ignore_british_spelling_licence, should_ignore_changelog_filename, should_ignore_notice_filename, should_ignore_licenses_directory_lowercase_and_uppercase, should_ignore_spdx_extension, plus the negative-side pins should_not_ignore_files_with_license_as_path_substring and should_not_ignore_files_with_embedded_license_or_changelog_substring to lock in that the base-name match only fires when the file is named exactly one of the variants).
• compression_level = "light" is now observable in the trace log (M18-W10, M33; r2 doc-accuracy fixes folded): the audit caught that a large PR going through apply_compression(Light) routinely emitted Large PR detected: 21 files, 724 changes, compressing at Light level (0 actions) followed by Split into 17 chunks — the (0 actions) token made the call look like a no-op even though Light had filtered out generated/vendor files (or compressed deletion-only files). Two structured tracing events are now emitted, both keyed by compressed_chunks = N: apply_compression reports files_before, files_after, compressed_chunks = before - after, and actions_count (the number of action strings) at the end of every call; apply_configured_compression reports the same plus configured, assessed, and effective_level so a "auto" diff that lands on Light is distinguishable from an explicit "light" configuration. The doc comment on apply_configured_compression clarifies that compression level never changes chunking strategy — the chunker (chunk_by_files / chunk_by_hunks / adaptive_chunk / semantic_chunk) and max_tokens_per_chunk are the knobs that resize the slices; max_chunks_per_expert is a routing quota (chunks per expert), not a slicing knob. Per-level semantics now match docs/code-audit-default.toml (and the docs/config-schema.md:188 description): configured "none" skips compression entirely; configured "light" compresses deletion-only files only; configured "medium" does "light" plus generated/vendor filtering, priority sort, and a 200-char line-truncate (the generated/vendor filter is the delta — deletion-only compression is already in "light"); configured "aggressive" does "medium" plus a 120-char line-truncate and a 40K token budget (apply_token_budget, called only from the Aggressive branch). The auto-Light path in apply_compression filters generated/vendor files only (no token budget — that lives in Aggressive); users who want a tighter prompt budget raise the level to medium / aggressive, while users who want to protect the chunking strategy while trimming only the deletion-only files use "light". New behavioural pins: test_apply_light_compression_no_removed_files_emits_zero_compressed_chunks (Light on a diff with no generated/vendor files: actions empty, files.len() unchanged, the tracing event still fires so the no-op is visible), test_apply_compression_none_emits_zero_compressed_chunks (None: trivial no-op with structured logging), test_apply_configured_compression_light_no_deletion_only_emits_zero_actions (explicit light with no deletion-only files: the audit's RENG-19 symptom — empty actions, structured tracing event with compressed_chunks = 0), test_apply_configured_compression_none_emits_zero_compressed_chunks, test_apply_configured_compression_medium_reports_compressed_chunks_count (medium removes both generated/vendor and deletion-only files: compressed_chunks = before - after, locked in by assert_eq!(removed, 2) on a Cargo.lock + saved/deletion-only fixture), and test_apply_configured_compression_auto_with_light_assessment_emits_zero_when_clean (30 plain source files with empty hunks → Light no-op).
• CLI --output wins over config.output_dir, and a shared helper deduplicates the ReviewOutput literal (M34, r1 + r2): the explicit file named by --output <path> is the absolute primary write target; when config.output_dir is non-empty, a timestamped copy of the same content is also written there by handlers::output::write_report_text. Both targets are now logged — the five Review dispatch arms (review --path (requires --local-path <repo>), review --mr-url, review --diff, review --local-path, review --stdin) trace --output at the CLI layer, and every ask / describe / improve / update_changelog handler traces both targets via a new shared helper log_output_targets. The previous ask / describe / improve handlers each repeated the 14-line ReviewOutput struct literal three times (9 sites in total); single_expert_review_output(expert_name, markdown) lives in src/cli/handlers.rs and absorbs all nine call sites. The same shape now applies to update_changelog (M34 r2: cli/handlers/changelog.rs was using an inlined literal and dropped the timestamped copy); the review --stdin arm also emits the dual-target trace so the dispatch layer and the helper trace read identically. A regression test (ask_stdin_with_question_succeeds_with_mock_llm, spawned with --config-dir <temp> + isolated HOME) pins the new dual-write: every ask / describe / improve / update_changelog run now lands exactly one review_<ts>.md under <state>/reports/, with content matching the stdout report (modulo trailing whitespace), and an unwritable output_dir makes these commands exit non-zero — consistent with review, and a silent drop would be a regression on the docs promise so the test does not paper over it. Side effect the docs now match: the dual-write documented in docs/getting-started.md finally applies to these four commands too (matches review / repo-review).
• The Verification pass line and the ## Injected Context section now render end-to-end in real runs (M35): run_experts_inner captures M31's rich VerificationStats (findings_checked / batches_total / batches_llm_failed / batches_parse_failed / batches_skipped / skipped / dropped) at the verifier call site and projects it onto the M32 simple {checked, skipped, dropped} shape via ConsolidatedReport::with_verification_stats. The full per-kind breakdown still logs through the existing Verification pass: INFO line; Some(_) here is what makes the report-side "ran on N batches, M skipped (parse failures / LLM errors / empty), dropped K" line appear. run_experts_inner reads AGENTS.md once up front via read_agents_md_meta_if_applicable(project_path, config) (same DEFAULT_MAX_FILE_BYTES cap as the CLI / server callers' mr_info.agents_md path, so the prompt body and the meta row cannot disagree; the Path::is_dir() guard on this helper short-circuits remote-server reviews — where mr_info.project_path is a group/project slug, not a directory — and the section is silently absent when the row list is empty). The changed-file rows are collected at the per-task build site: every per-task prompt body is now built by build_file_contents_section_with_meta (the body-only build_file_contents_section was deleted, so future readers can't reintroduce the body/meta drift), each task emits one InjectedContextEntry per file it injected, and dedup_injected_entries collapses the hunks-strategy case where one file is split across several chunks (one row per source in the audit table). The end-to-end proof (team::orchestrator::tests::run_experts_wires_verification_stats_and_injected_context_into_report) drives run_experts with a real local checkout (AGENTS.md + changed file) + mock LLM (parseable expert YAML, verdicts: [] for the verifier), populates both fields on consolidated, and surfaces both **Verification pass**: and ## Injected Context in the rendered lead summary. M35 also closes three M32 r2 residuals (P3-2/P3-3): displayed = injected_content.trim_end() is computed once at the top of the per-file loop and shared by the budget check, injected_bytes, sha256, and the body (previously the body was trimmed but the hash/count used un-trimmed — embedded slice and fingerprint off by trailing newlines); InjectedContextEntry doc softens the two narrow cases where original_bytes < injected_bytes is possible (20 001–20 047 byte truncation-band on changed files; lossy-UTF-8 expansion on AGENTS.md) and the percentage cell renders the literal ratio rather than clamping to 100 % (status field reads the truncated flag, not the byte ratio); test_omitted_file_has_no_meta_row pins the P2-2 "omitted file must NOT carry a row" invariant end-to-end. Plus the M34 r2 follow-up: every review local / MR / stdin / repo-review handler (run_stdin, run_mr, run_local, run_local_repo, run_local_path, run_repo_review_local_or_enhanced) now calls log_output_targets alongside the dispatch-arm --output trace, so logs.ndjson carries the dual-target shape the M34 r2 audit catalogued as missing for the review handlers. And the log-line noun is unified: pipeline.rs coverage logs renamed from Coverage: X of Y files reviewed to File routing: X of Y files assigned to a successful expert task, matching the M32 W4 renderer header — one noun on every surface. New tests: run_experts_wires_verification_stats_and_injected_context_into_report, dedup_injected_entries_collapses_repeated_source_into_single_row, read_agents_md_meta_if_applicable_skips_remote_slug_paths, read_agents_md_meta_if_applicable_respects_inject_agents_md_off, test_render_lead_summary_verification_stats_mislabels_skip_causes, test_meta_row_hash_and_bytes_match_trimmed_body, test_omitted_file_has_no_meta_row, injected_entry_render_row_handles_injected_greater_than_original.
• The security expert separates mechanical secret-leak hits from semantic security issues, and the score no longer drops on a sea of false positives (RENG-114): the repo-review path used to deduct 8 points per regex hit (capped at 20) for api_key = "…", password = "…", sk-… and -----BEGIN … PRIVATE KEY----- patterns, and nothing else — so a repo with 7 mechanical hits (fixtures, format templates, illustrative values) reported security: 44/100 and dragged the rest of the report. After the rework the expert emits every hit with a [mechanical] | [semantic] kind tag, the summary breaks the count down by kind, and the score uses two separate formulas: mechanical hits deduct 4 per hit (capped at 8 hit × 4 = 32 max — the regex can never drive the score into Critical on its own), while semantic hits deduct 12 per hit (capped at 8 hit × 12 = 96 max). The static sweep has no semantic producer in-tree yet, so the path is reserved for a future LLM security expert (the catalogue in Security::check_items() already names the four RENG-114 areas: output_channel_redaction, proxy_bypass_evasion, temp_file_permissions, token_in_query_or_path) and is the single source of truth for prompts that want to render them.
• Test fixtures and bracket-wrapped placeholder values are excluded from the mechanical scan (RENG-114): the seven-hit pattern-matched repo had five false positives — four in tests/ and _tests.rs paths, one wrapping the value in <…> / 〈…〉 for documentation. scan_security_patterns now skips every entry whose path matches experts::facts::is_test_file (the same heuristic that drives the test-coverage and code-organisation classifications, so a file the system already calls "a test" is uniformly a test), and scan_security_patterns_in_text drops any matched value whose span contains < / > / 〈 / 〉. The fixture/placeholder filters live at both layers (entry and line) so a value the regex never sees cannot leak past either. End-to-end with the published binary: identical payloads in tests/fixtures.rs, src/upload_tests.rs, src/__tests__/helper.ts, src/output/parser/tests.rs produce no findings; the same payload in src/upload.rs produces one [mechanical] finding; security scores 96/100 (100 − 4) on that single hit.
• Fallback experts no longer enter the weighted aggregate or the report's Top-Risks list (RENG-115): an expert whose fallback = true (e.g. a CodeQuality chunk whose LLM call failed after every retry, returning the LLM_FALLBACK_SCORE = 70 placeholder) used to claim a slot in the headline math and in the per-expert risk ranking. Three layers now agree: weighted_total filters fallback out before compute_weighted, the aggregator's top_risks does the same so a placeholder cannot pollute the per-expert list, and total_weight_f (the divisor build_score_breakdown uses) sums only non-fallback weights so the per-row weighted_contrib values add up to the headline health_score even when a fallback is in the mix. build_score_breakdown itself drops fallback rows; the invariant is pinned by build_score_breakdown_sums_to_headline_even_with_a_fallback. All-fallback inputs now honestly report 0/100, Critical (no genuine data → no real score), and the regression test suite simulates a mixed set to lock the math.
• code_style no longer drops to 0/100 or grabs a Top-Risk slot for missing style configs (RENG-115 / RENG-117): a repo with no .editorconfig / .rustfmt.toml / .clippy.toml / mypy.ini / ruff.toml used to score 0/100 (because every missing config widened the denominator) and grabbed the top of the report's Top-Risks list — a hygiene problem dressed up as a code-quality defect. The expert now scores 100/100 whenever the static sweep has no genuine style defects to report; missing-config findings stay surfaced in the details list as [hygiene] Missing … notes (severity: note, with a concrete recommendation) so the user still sees them, but they live in a separate channel that the risk ranking ignores. The report-layer filter (scoring::is_hygiene_finding + expert_is_hygiene_only) makes build_risk_categories and pick_top_risks skip an expert whose every detail is a hygiene reminder; pick_top_risks also double-checks via the same predicate as defence-in-depth against any future caller that hands in raw categories. End-to-end pin: report_top_risks_for_unconfigured_repo_excludes_code_style (local path) and full_path_top_risks_drop_hygiene_and_fallback_experts (full/LLM path through build_output_from_aggregated, with two explicit-fallback experts added so the run mirrors an unreachable-LLM endpoint) both assert code_style never appears in the Top-Risks output and the Risk Map / Top Risks surfaces in one report agree on the same set of areas.
• The full/LLM path's Top Risks no longer bypasses the hygiene filter (RENG-115 round 3): r2 surfaced that build_output_from_aggregated (src/actions/repo_review/mod.rs:148) was still pulling top_risks from aggregator::aggregate's "5 lowest merged experts" list — fallback-filtered, but not hygiene-filtered — so a full/LLM review on the same no-config repo could still emit 3. code_style (100/100) under Top Risks while the report's own Risk Map omitted it, and the Prioritize: … recommendation repeated the pollution. The aggregator's own top_risks is left alone (other consumers may still want "5 lowest experts"); the report assembler's conclusion switches to the same pick_top_risks(&risk_categories) source the local path uses, so the two surfaces now agree by construction. The matching "Prioritize:" line is rebuilt from the same filtered list (build_conclusion_recommendation mirrors the aggregator's wording so the two paths read identically); a Top Risks list that comes out empty after hygiene filtering lands on No significant issues found. instead of Prioritize: .. Pinned by full_path_top_risks_drop_hygiene_and_fallback_experts (the r2 three-run reproducer captured as a single test: built the same AggregatedResult shape, asserted top_risks ≡ risk_categories areas, no hygiene-only or fallback expert in either surface, recommendation clean) and build_conclusion_recommendation_matches_prioritize_shape (wording-stable across empty / non-empty inputs).

Fixed

• SecretHit and PublishRefusal no longer derive Debug with the plaintext value (M25 self-audit top-1, M27): both types used #[derive(Debug, …)], so any tracing / anyhow! / panic! payload that handled a hit by {:?} printed the credential verbatim — a leak vector that survived RENG-113 (which closed the report and publish channels but left the error / log paths exposed). The module's own docstring promised the opposite: "Debug only shows fingerprint + length". Both types now carry hand-written Debug impls that emit kind, key (when present), the value's len (in chars), the 8-hex-char fingerprint prefix, and the byte range — never the plaintext. PublishRefusal delegates to SecretHit's safe form, so format!("{:?}", refusal) is safe by construction and matches the operator-visible form format_publish_refusal already prints. Pinned by secrethit_debug_does_not_leak_plaintext_value (asserts every expected redacted field is in the Debug and the plaintext isn't), secrethit_debug_value_substring_can_not_appear (a value whose first 8 chars look like a fingerprint must not be echoed as a substring), and publishrefusal_debug_does_not_leak_plaintext_values (the multi-hit refusal form).
• JSON-escaped quoted credentials no longer slip the publish-time gate (M18-W14, M27): a raw LLM response that quoted a credential as {\"password\": \"hunter2_real\"} (the JSON-escape form) used to fall through the structured detector — the value char class excluded ", so the regex started capturing at the single \ (below MIN_VALUE_LEN) and the key/value token reached the publish path unmasked. scan_secrets now pre-normalizes \" → " before running the detector set, and the six structured-assignment patterns wrap the key in optional ["']? so JSON-style "key": "value" matches the same regex as TOML-style key = "value". The hit range is mapped back into the original text (a per-byte position map is built alongside the normalized string) so redact_text substitutes over the right bytes — the surrounding JSON framing survives intact. Idempotence (redact_text(redact_text(x)) == redact_text(x)) is preserved on the escaped form: the first redaction produces a marker, the second is a no-op. Coverage: redact_text_catches_json_escaped_value (the {\"password\": \"hunter2_real\"} shape, asserts plaintext gone + kind=password + key=password + framing preserved), scan_for_publish_catches_json_escaped_value (the publish gate sees the same hit), redact_text_is_idempotent_on_json_escaped_input (idempotence on the escaped form), scan_secrets_ranges_map_back_to_original_bytes (a unique-marker value confirms &body[hit.range] slices the original bytes, not the normalized form, and the redacted output keeps the JSON framing around the placeholder), redact_catches_value_only_inside_json_escaped_object (a glpat-… token inside a JSON-escaped description field is masked through the value-only path), unescape_quotes_does_not_strip_lone_backslash / unescape_quotes_does_not_strip_other_escapes (the normalizer only collapses \"; a trailing lone \ or \n is left as-is, matching the finding's bounded instruction to not relax other rules). All pre-existing RENG-113 round-1/2/3 redaction tests (scan_does_not_re_match_the_redaction_marker_itself, redact_text_is_idempotent, publish_gate_accepts_a_board_redacted_from_false_positives, the round-3 publish_review_passes_through_a_rolled_up_credential_title and board_section_redacts_titles_in_the_rolled_up_list end-to-end pins) still pass.
• Empty LLM completions are auto-retried with disable_thinking=true instead of advancing the chain (M26): a reasoning model that burns its max_tokens budget on reasoning_tokens (e.g. mimo-v2.5-pro, deepseek-v4-flash) returns 200 OK with an empty choices[0].message.content. The previous loop gave up on the provider immediately, dropped the chunk to a 70-fallback, and never tried the cheapest mitigation the config already supported. The M24 recall run hit this 37 times in 18 instances — every code_quality chunk failed — and 6 bug-file-bearing chunks were never assessed by the model (T1 recall = 0/18's head structural cause, .tower/comms/findings/20261010-tower-bug-chunk-completion-38.md). M26 retries the SAME request once with "thinking": {"type": "disabled"} before advancing the chain. The retry is bounded to ONE attempt per provider (a reasoning model that ignores thinking: disabled will answer empty again, and the chain advances), it skips when the config already has disable_thinking = true (no useful second attempt), it does not run on 5xx (the backoff retry budget owns those), and it carries the same LLMCallSample row so the LLM page's call-level history still surfaces the diagnosis. The disable_thinking request body (src/llm/provider.rs::build_openai_compatible_body) and the EmptyCompletion verdict (src/llm/client/mod.rs) were already in place — the change is the per-provider retry wiring between them. End-to-end: M24's 18/18 instance set had 37 empty completions against the same single chain head; under M26 the cheap fix runs before the chain gives up, recovering real content where the model honours thinking: disabled and still falling back to the chain where it doesn't. Pinned by empty_completion_triggers_a_disable_thinking_retry_that_recovers (the happy path — scripted empty + scripted success on the second try, with the recorded disable_thinking flag history [None, Some(true)]), empty_completion_with_disable_thinking_already_set_does_not_retry (no useful second attempt — same empty payload, asserts samples.len() == 1), empty_completion_retry_is_bounded_to_one_attempt_when_still_empty (the recovery doesn't help shape: 2 attempts only, no third), empty_completion_retry_only_fires_on_empty_verdicts_not_on_5xx (5xx still uses the 3-attempt backoff budget), and empty_completion_retry_advances_to_next_provider_when_still_empty (the chain advances and the second provider gets its own retry budget — sequence [None, Some(true), None, Some(true)]). The pre-existing an_empty_completion_is_an_error_and_advances_the_chain test was rewritten to match the new contract (the 1 → 2 sample count, the 2 → 3 with the chain advance), with the comment block explaining exactly which line of the assertion changed.
• --verbose dump files no longer bypass RENG-113 redaction or its 0600 perms (M18-W1): the report markdown and the publish path have been OpenOptions::mode(0o600) + redact_text since RENG-113 round 2, but the <output>.raw/ and <output_dir>/review-raw/ dumps still went through std::fs::write under the user's umask — files landed at 0664 and the directory at 0775, so the raw LLM exchange (which can quote a password = "…" fixture exactly the way the report markdown can) was readable by every other account on the host. team::orchestrator::create_secure_dump_dir now chmods the dump directory to 0700 (matching the 0600 file target) and write_secure_dump_file routes the dump body through redact_text before the OpenOptions write — the dump-side counterpart of RENG-113, scoped to src/team/orchestrator/pipeline.rs:328-349 and src/cli/handlers/review.rs:13-26. Pinned by create_secure_dump_dir_lands_at_0700 (chmods back from a 0755 pre-existing mode to confirm the path is exercised), write_secure_dump_file_lands_at_0600 (same belt-and-braces for the file, including the truncate-on-existing-overwrite chmod), write_secure_dump_file_redacts_credential_shaped_values (a password = "hunter2_real" body round-trips with the value replaced by the ***[redacted:kind=password,key=password,len=12,fp=…]*** marker the report uses, key + unrelated lines untouched), and create_secure_dump_dir_returns_none_on_failure (the fail-soft contract — a CREATE error degrades to None, never panics).
• The verifier's batch-level skips are now counted (M18-W7): the optional verification pass was fail-open on LLM and YAML errors but said nothing about them — the report's "Verification pass" line read "checked N, dropped 0" whether the verifier ran clean or every batch silently passed through. A new team::verifier::VerificationStats struct now tracks findings_checked / batches_total / batches_llm_failed / batches_parse_failed / batches_skipped (syntactically-valid-but-empty verdict list, kept fail-open) / skipped (sum of the three failure kinds) / dropped (findings the LLM told us to drop). verify_findings keeps its legacy Vec<DroppedFinding> return so the src/actions/repo_review/mod.rs:496 call site (out of this mission's scope, owned by M26) is unaffected; the orchestrator goes through the additive verify_findings_outcome variant. The stats are log-only in this release — the orchestrator surfaces them in the Verification pass: INFO line — and the report-level "ran on N batches, M skipped (parse failures / LLM errors / empty), dropped K" line is deferred to the M35 follow-up that threads the struct onto ConsolidatedReport (per the M31↔M32 ruling, which holds the renderer wiring out of scope for both M31 and M32). Pinned by test_verify_stats_track_clean_run, test_verify_stats_count_llm_failures, test_verify_stats_count_parse_failures, test_verify_stats_count_empty_verdict_list_as_skipped (a verdicts: [] body now counts as a skip rather than a quiet keep-all), test_verify_stats_aggregate_mixed_outcomes (11 findings across 2 batches — one drop + one empty verdict list → dropped=1, skipped=1), and test_verification_outcome_default_is_empty (the LLM-configs empty early-exit shape).
• The lead overview acquires a permit on the same semaphore as the expert pass (M18-W11): the audit expected a wall-clock saving (the lead overview running in parallel with the first expert batch), but the overview is a strict serial prerequisite of the expert pass — global_context.clone() is fed into every expert task before join_all, and the permit therefore has no other holder when it is acquired. The change is consistency / future-proofing, not latency: a future restructure that lets experts run with an empty overview first and re-prompt once it lands will get the cap right automatically. The semaphore is now built before the overview call (build_concurrency_semaphore is the new pre-overview helper; setup_concurrency_control keeps its existing shape for the post-overview call). The 0-cap backstop is preserved (RENG-107 r2) and re-tested at the new site (build_concurrency_semaphore_never_builds_a_zero_permit_semaphore). No wall-clock saving is claimed.
• parse_global_review_context no longer floods logs at WARN (M18-W12): the lead overview's strict / sanitized / fenced-block YAML parser degrades to None when every attempt fails, and that None is the expected fallback — but it used to log at WARN, the same level real LLM-call failures log at, and NAS 24 days showed 94 such WARNs, 100% of the team pipeline's WARN budget, drowning the genuinely-informative errors. The log line is now INFO with a structured reason_code field: tab_indent_fallback_succeeded / fence_fallback_succeeded (so log pipelines can count fallback successes) / schema_mismatch (the final-None downgrade, with the strict parse error inlined). The prompt-build and LLM-call failures keep their WARN level. Pinned by parse_global_review_context_logs_reason_codes_not_warnings — captures the call's tracing output via tracing::subscriber::with_default (a thread-local subscriber, not the process-global one, so it cannot be silently shadowed by another module's RENG-70 install) and asserts both WARN is absent and INFO + reason_code=schema_mismatch is present; a revert of the level or the reason_code fails the assertion immediately. No call-site change required — the expert pass continues without a global context exactly as before.
• The adjudicator's 0 candidates log line now explains itself (M18-W6, pipeline side): when adjudicate_min_severity is wider than what survived lead-consolidation (e.g. low while every finding was confidence-downgraded to Severity::Note / rank 0 by lead_consolidator), the pass silently drops nothing and the report reads as "adjudication ran, dropped 0" — the audit's "adjudicate 开了但什么都没做" silent-drop pattern. The new log line includes the count of findings the consolidation pass demoted below the gate and the exact knob the operator can turn (adjudicate_min_severity = "note"); the consolidator-side semantic change (raising min_confidence to keep findings at their reported severity) is owned by M32 and independent of this hint.
• The verifier and the adjudicator now share one verdict-list parser (M18 self-audit #5): verifier.rs::parse_verdicts and adjudicator.rs::parse_verdicts were near-identical copies of the same clean_yaml → strict → fenced fallback → pull verdicts: list routine, with only the decision shape differing (verifier maps to {index, drop, reason}; adjudicator maps to {index, verdict, new_severity, reason, cited_lines}). The shared routine lives in team::verdict::parse_verdict_entries returning a RawVerdictEntry { index, verdict, new_severity, reason, cited_lines }; the verifier maps it to {index, drop, reason}, the adjudicator to {index, verdict, new_severity, reason, cited_lines} with the existing false_positive / false-positive / drop / downgrade mapping preserved. The shared routine's error wording is the verifier's ("verdict YAML parse failed"); the adjudicator's old wording ("adjudication YAML parse failed") was a logging-observable side change — no test pinned it (test_parse_verdicts_malformed_errors asserts only is_err()), so the wording change is observable in the WARN text but not pinned by a test; it is a logging nit, not a behaviour break. The five verifier contract pins (test_parse_verdicts_fenced_yaml / test_parse_verdicts_plain_yaml_without_fence / test_parse_verdicts_missing_fields_default_to_keep / test_parse_verdicts_tolerates_surrounding_text / test_parse_verdicts_missing_verdicts_key_errors) all still pass against the new shared entry, and the four adjudicator-side pins (test_parse_verdicts_all_kinds / test_parse_verdicts_unknown_verdict_defaults_to_confirmed / test_parse_verdicts_malformed_errors / test_parse_verdicts_invalid_new_severity_is_none) do too. Pinned at the shared helper by parses_fenced_yaml_with_full_fields, skips_entries_without_an_index, missing_verdicts_list_errors, malformed_yaml_errors.
• Inline #[cfg(test)] mod … { … } blocks inside regular .rs files no longer fire the mechanical secret-leak regex (RENG-114 follow-up, M19): the RENG-114 fix closed the same false positive for files named tests.rs / *_tests.rs / under tests/ / __tests__/, but a much more common pattern — inline test modules inside production-named files — still got hit. Sirena's crates/sirena-cli/src/upload.rs is the reproducer: 4 password: "sk" assignments at lines 673 / 741 / 843 / 994, every one inside the file's #[cfg(test)] mod tests { … } block, and upload.rs is the implementation file (not a test file by name). Without this layer the security expert emitted 4 [mechanical] Hardcoded password findings and dropped the security score by 16 points (4 × MECHANICAL_PER_HIT) on a repo with no real leak. experts::facts::cfg_test_line_spans is a pragmatic containment heuristic: it finds every #[cfg(test)] attribute on its own line, skips any companion #[…] attributes, expects a mod <name> { opener on the next line, and walks braces until depth returns to 0. r2: the brace counter is now a small per-line state machine — count_code_braces — that tallies braces only outside string literals ("…"), char literals ('…'), line comments (// …) and block comments (/* … */), with both in_block_comment and in_string threaded across line boundaries. The audit proved this was needed: the r1 byte-only counter, given a single assert_eq!(brace("{"), 1); inside a test, reported span (2, 12) for a module that really closes at line 8, swallowing every following production line (a real-leak false negative — the security sweep never sees a credential on the suppressed lines). Raw string literals r"…" / r#"…"# and the byte / c-string prefixes are recognised (the #s are counted and the matching "<#×N> is found; inside a raw string the backslash is literal, so a \" does not close the literal). Multi-line strings are tracked via the in_string state (a "…\" at end of one line keeps the string open on the next). r3 — Rust lifetimes must not be parsed as char literals: the r2 state machine treated every ' as a char-literal opener, so a &'static str / '_ / <'a> started a bogus literal scan that consumed the line's {, under-counting opens, the depth-0 break fired at an inner }, and the cfg-test span ended early — the RENG-114 false positive came right back. The reviewer-facts r2 audit caught this on a compiled-harness span run: spans ended at (661, 1868) / (672, 770) / (475, 836) / (614, 1203) where the real closes are 1909 / 1549 / 1042 / 1257 (sirena still passed the acceptance surface only because the first lifetime line — 1181 — happens to sit after the four offending fixture lines ≤ 994). The r3 rule: a ' opens a char literal only when the pattern is '\…(escape) or''(exactly one byte, then a closing'); any other shape — '(incl.'_), '— is a lifetime or stray apostrophe and the scanner consumes the'+ following identifier without entering a literal state. With this in place, the 4-file audit closes at the real column-0}(546 / 1909 / 1549 / 1042) and sirena closes at 1257; the minimalassert_eq!(brace("{"), 1);reproducer stays at span (2, 8) and the RENG-114 false positive does not return.scan_security_patterns_in_textconsults the span list once per file and skips every line that falls inside a cfg-test block; the entry-level test-file skip and the inline cfg-test skip compose, so production lines that carry the same payload in the same file still surface. **r2 fixture fix**:scan_security_patterns_in_text_skips_cfg_test_blocksnow uses 4 password-shaped fixture lines (one pert.password = …/password: "sk".into()/password: "sk".to_string()/let password = "sk";— three of the four forms sirena'supload.rsactually carries), so a revert of the skip surfaces exactly 4 findings (one per line), not 1. Pinned bycfg_test_line_spans_finds_a_single_block, cfg_test_line_spans_handles_nested_braces, cfg_test_line_spans_handles_multiple_attributes_between_cfg_and_mod, cfg_test_line_spans_finds_multiple_blocks(the four facts-layer pins from r1),cfg_test_line_spans_ignores_braces_inside_string_literal(the r2 reviewer-facts counterexample:assert_eq!(brace("{"), 1);does not extend the span past the real close at line 8, and the productionapi_keyline 10 remains visible to the security sweep),cfg_test_line_spans_ignores_closing_brace_inside_string_literal(the}-in-string mirror), cfg_test_line_spans_ignores_braces_inside_line_comment, cfg_test_line_spans_handles_multi_line_block_comment(thein_block_commentstate threading),cfg_test_line_spans_handles_escaped_quotes_in_string_literal(the"/\pair rule),cfg_test_line_spans_handles_line_continuation_string(the<newline>continuation across lines),cfg_test_line_spans_handles_raw_string_with_escape_chars(ther#"…"#form with"inside the body), andcfg_test_line_spans_does_not_overrun_on_real_source_files(the r3 audit (tightens the r2 trim() check to a column-0}literal so an indented inner}does not pass; loads the 4 audit files and asserts the end-line is the mod's real}at column 0; verified to fail withis_escape = trueregression —src/publisher/mod.rs: last span ends at line 1868 (" }")andcfg_test_line_spans_treats_lifetimes_as_lifetimes_not_char_literalsboth fail), andcfg_test_line_spans_treats_lifetimes_as_lifetimes_not_char_literals(the r3 reviewer-facts counterexample:&'staticinside an inline test module must not be misread as a char-literal opener; span stays at (1, 8) and the in-testpassword = "sk"line is suppressed). The analysis-layer pins arescan_security_patterns_in_text_skips_cfg_test_blocks(the sirena shape, 0 findings) andscan_security_patterns_in_text_keeps_production_lines_outside_cfg_test(a production-lineapi_key` in the same file still reports).
• test_coverage 70/100 returns to Top Risks and Risk Map; code_style (the [hygiene]-tagged expert) still does not (RENG-115 follow-up, M19): the RENG-115 round-3 logic accepted either severity == "note" OR a [hygiene]-tagged message as the hygiene signal, and the broadened predicate swallowed test_coverage on sirena — its only emitted detail is the severity: note "No CI test step detected" line, and the 70/100 expert vanished from both visible surfaces while still showing 70/100 medium, contribution 14.0 in the breakdown table. is_hygiene_finding now matches only the explicit [hygiene] prefix the producer stamps on its reminders (today: code_style for missing-config notes); a bare note severity is no longer enough to filter an expert out of build_risk_categories and pick_top_risks. The contract pins: hygiene_predicate_accepts_only_tagged_findings (the predicate itself), note_only_test_coverage_stays_in_risk_categories_and_top_risks (the sirena reproducer: test_coverage 70/100 + single note detail stays in both risk_categories and top_risks, while code_style 100/100 with three [hygiene]-tagged notes stays out — the two cases that drove the original design remain in opposite buckets, with the predicate narrower than before). The RENG-115 round-3 / reworked-M15 pins (build_risk_categories_drops_hygiene_only_experts, report_top_risks_for_unconfigured_repo_excludes_code_style, full_path_top_risks_drop_hygiene_and_fallback_experts) all still pass: code_style's missing-config findings carry both the [hygiene] tag and severity == "note", so the narrowed predicate still classifies them as hygiene, and the genuine-risk experts in those tests already carry a non-note finding so the new predicate leaves them alone.
• The rolled-up board section now masks Finding::title (RENG-113 round 3): round 2 closed every unredacted channel in the report and publish path except one — InlinePlan::board_section (src/publisher/policy.rs:420) still interpolated finding.title verbatim for findings the policy admitted but withheld from inline delivery past the per-round cap (and the docs-only listing). The reviewer reproduced the dead-end: three admitted findings with a credential in the third title carried the plaintext into the board, the publish-time gate refused the whole publish (fail-closed, nothing leaks), and the operator had no escape — the per-expert section already masked the same title, so the visible report carried no plaintext yet the gate refused. Fix: one-line crate::models::redact_text(&finding.title) at the rolled-up render site (same fix family as the round-2 channels), plus two new tests: board_section_redacts_titles_in_the_rolled_up_list pins the marker form (kind=password, anchor, severity verbatim) and board_section_redaction_lets_a_rolled_up_credential_pass_the_gate runs the production plan_inline_notes → render_board → scan_for_publish path and asserts the gate passes. The end-to-end publish_review_passes_through_a_rolled_up_credential_title test in lib.rs::publish_policy_e2e_tests goes against a mock provider and pins the operator-visible contract: the board carries the marker, two inline notes still go up, and the gate never refuses on its own rolled-up output.
• Round-2 test doc comment corrected (RENG-113 round 3 P3 nit): test_render_repo_review_markdown_redacts_password_in_detail_message claimed round 1 "missed message" — render_detail's message was already redacted at d5fc1d4 (r1). The test still adds useful coverage; only the historical claim was wrong.
• The publish-time gate and the redaction marker now compose (RENG-113 round 2): round 1 added a credential gate that ran scan_for_publish on every body about to be posted, but the gate had no concept of the redaction marker ***[redacted:…]*** — the marker's kind=…,len=…,fp=… body was itself a key = value line, and the structured-assignment detector happily matched ***[redacted:kind=password up to the first comma. Result: any round whose board carried a marker was refused by its own gate (the user's api_key = process.env.API_KEY-style false positives fired the same way: redaction masked the line, the marker re-fired the gate). The fix is in src/models/secrets.rs::marker_spans (a single regex pass finds every marker span first) and scan_secrets drops any hit whose value range starts inside one — gate and idempotence (redact_text(redact_text(x)) == redact_text(x)) are restored in one place. Coverage: scan_for_publish(&redact_text(x)).is_ok() per pattern class (5 classes), redact_text_is_idempotent over a multi-credential body, publish_review_passes_through_a_masked_board_with_credential_in_summary (the production render_expert_markdown → render_board → scan_for_publish chain), and publish_review_passes_through_a_masked_board_with_credential_in_title. The round-1 e2e tests (output_with hand-built markdown) still pass: a real plaintext hit still refuses, and the rendered-board path is now proven through the production renderer.
• Finding.title and the dropped-findings appendix are redacted (RENG-113 round 2, P2): round 1 covered Evidence / summary / impact / recommendation / TL;DR / lead-summary fields / raw-LLM-response excerpt but missed the four channels the reviewer's probe (/tmp/reng113-probe/rust-probe) showed reaching the report file verbatim: Finding::title in render_expert_markdown, render_team_report_with_scoring and render_aggregated_markdown; the verifier's reason and the dropped finding's title in render_dropped_findings_appendix and its with_state siblings; overview.lead_summary (rendered raw at repo_review/render.rs:129 while s.summary on line 154 was redacted — same inconsistent drift the reviewer spotted) and conclusion.recommendation (render.rs:213). All now flow through crate::models::redact(). New tests pin the title channel across the team / per-expert / aggregated paths (render_team_report_redacts_finding_title) and the appendix (render_dropped_findings_appendix_redacts_title_and_reason), plus the repo-review lead_summary / conclusion.recommendation paths.
• format_inline_body masks title and recommendation at the source (RENG-113 round 2): the publish-time gate scans the inline note body too (defense in depth on top of the board's redact pass). Round 1 missed that format_inline_body interpolated raw title and recommendation fields — a finding whose title quotes a config line would have been masked in the board body but posted verbatim to the MR discussion. Round 2 routes both fields through crate::models::redact_text in the publisher itself, so the gate sees a body the publisher has already masked. New test format_inline_body_redacts_title_and_recommendation pins the contract; the round-1 inline-note refusal test was rewritten to exercise the gate's still-live defense-in-depth check (the renderer bypass scenario — a hand-built board whose title the redactor never sees still refuses).
• Report files are created with 0600, not chmod'd after the fact (RENG-113 round 2, P3): round 1's fs::write then set_permissions left a transient window where the file briefly existed with the user's umask perms (typically 0644), and an interrupted run left it that way. Round 2 uses OpenOptions::create(true).write(true).truncate(true).mode(0o600) (Unix-only via OpenOptionsExt) so the file is born at 0600. The chmod after-step stays for the overwrite path: pre-existing files don't get their mode reset by truncate. Two new tests pin the contract: write_report_text_overwrites_an_existing_file_at_0600 (the truncate path inherits the mode if no chmod runs) and write_report_text_tightens_a_pre_existing_permissive_file (a file dropped on disk with 0644 is brought back to 0600).
• Doc / impl drift, dead code (RENG-113 round 2, P3): the secrets.rs module header promised "any sufficiently long hex/base64 string" is detected — no such pattern exists; the catalogue is the surface. The module header now states it. The marker fp is 16 hex chars; format_publish_refusal prints an 8-hex prefix that greps the marker. The CHANGELOG and the refusal-formatter comments are corrected to "16-hex-char SHA-256 fingerprint" (the marker form) and "8-hex prefix" (the error form). The unused MIN_PUBLISH_HIT_LEN is removed (it duplicated MIN_VALUE_LEN). PublishRefusal.body is removed: surfacing the rendered text in an error path was itself a leak vector — any Debug-printing logger / formatter that handled the error type printed the plaintext. The fingerprint is the join key: an operator greps the local file for the 8-hex prefix and lands on the offending marker.
• Compliance verdict parsing now tolerates every URL shape the model picks, and the compliance LLM call has a typed-decode failure chain with a bounded per-ticket fallback (M42, r1 + r2 + r3 + R4): the M41 live smoke on real mimo-v2.5-pro surfaced two HIGH findings — every MR66 ticket parse-errored (ticket missing from response, because the model picked the GitLab REST API URL as the JSON key, /api/v4/projects/isletspace%2Fsirena/issues/73, while the parser only matched r.url (the work_items URL) and r.source (#N)); and MR68's full 3-ticket LLM call returned upstream error decoding response body and the section gracefully degraded to "3 ticket(s) fetched but not analysed" with 0 structured rows. Five changes close the loop:
  • Parser key / fence tolerance — parse_compliance_response now tries every plausible key shape a model might pick: the canonical work_items URL (with and without trailing slash), the GitLab REST API URL (/api/v4/projects/.../issues/N), the GitHub REST URL (/repos/o/r/issues/N or the GHE shape), the bare iid as a string, and the surface forms (#N / !N); extract_json_object now strips language-tagged fences (```json / ```JSON / 4+ backtick outer fences) before the brace walk; the system prompt pins the JSON key to the canonical work_items URL with an explicit example (defence in depth). Real mimo body shapes captured from the M41 run are desensitised into tests/compliance/main.rs fixtures — m41_captured_mimo_output_is_parsed (the captured shape now parses), m41_captured_output_would_have_failed_before_fix (regression guard against reverting to r.url- / r.source-only matching), m41_captured_output_with_outer_text_fence_is_parsed (the renderer-wrapped-capture fence is also stripped), and m41_captured_output_round_trip_end_to_end (full run_for_review wiremock round-trip).
  • Typed JsonDecodeError marker — a 200 OK + non-JSON body no longer surfaces as the opaque error decoding response body; the typed marker (status only — r3 dropped the body-excerpt capture after the r2 resp.bytes() approach failed on the live wire; see reports/compliance-fix-resmoke/REPORT.md §9.1 for the root-cause explanation: the M42-R2 first-attempt live regression was the &body_str[..4096] cap clipping every response > 4 KiB before the parse step) classifies as Unknown in retry_verdict, so the existing backoff retry applies, and the compliance sub-step downcasts the anyhow chain and surfaces the failure in Notes as one of JSON decode: / empty completion: / <original message> (rendered through redact_text).
  • finish_reason-driven per-ticket fallback — CompletionResult (src/llm/provider.rs) carries finish_reason: Option<String> (OpenAI's choices[0].finish_reason, Anthropic's stop_reason); when a batch call lands Ok(...) with finish_reason ∈ {"length", "max_tokens"} AND valid_checks < fetched.len(), run_per_ticket_fallback runs up to MAX_TICKETS_PER_REVIEW (= 3, declared at src/models/config.rs:342) single-ticket calls, each with its own fresh budget; make_parse_error_check is the placeholder helper that builds a TicketCheck shaped like a parse failure so the renderer labels the row consistently regardless of which path produced it. stop does NOT trigger (the over-eager-fallback guard).
  • Err-arm pre-check (r4 R-1 wires the M42-R2 deferred path) — the same bounded per-ticket fallback now also fires on the Err arm for retryable failures; the pre-check uses LLMClient::http_status_code (pub(crate), walks the anyhow chain): 408 / 429 / 5xx retryable, other 4xx permanent (no permanent-4xx budget burn — exactly the RENG-35 credential-problem anti-pattern the r3 round's pre-check had), no-status (transport / DNS / TLS) retryable. Cost bounded at MAX_TICKETS_PER_REVIEW extra calls on top of the failed batch call.
  • Per-ticket ref alignment + scheme derivation (the r3 secondary fixes) — the per-ticket fallback resolves against the fetched ticket's own ref (f.ticket_ref) instead of ticket_refs[i] by position (positional alignment mis-aligned after any earlier fetch failure); and gitlab_opposite_scheme_work_items_url derives the inverse shape in the source scheme (the r3 audit found gitlab_api_url_for_ref hard-coding https:// missed the http://…work_items/… variant on the wiremock fixture where the gitlab mock is http:// and the LLM body is https://).
  • End-to-end evidence (reports/compliance-fix-resmoke/) — M42 live resmoke on MR66 (raw/mr66-closes-73.md, binary sha256 48839e2592fbe5cd22d2c74904ee61a2ea9b240b30ec69b0380fd0150faecfce, 12m 33s): 1 structured verdict, zipformer-CTC surfaced under **Not satisfied**, matching pr-agent's baseline. M42-R2 live resmoke on MR68 (raw/mr68-closes-75.md, binary sha256 9c80f4d6…39d0, 10m 24s): 3 tickets analysed, 5+6+9 satisfied bullets with file:line evidence (ground truth from the raw capture — #75 = 5, #76 = 6, #77 = 9), 0 not_satisfied, 3+2+4 needs_verification; Notes carry only description references 8 tickets, capped at 3 — no JSON decode, no compliance LLM call failed. CI's exact clippy command (cargo clippy --all-targets -- -D warnings) is green (the r4 R-1 fix removed an orphan-doc-comment warning the r3 round left behind). The full set of compliance-integration pins: finish_reason_length_triggers_per_ticket_fallback (truncated batch + permanent per-ticket → fallback fired, Notes name truncation + fallback, LLM call count in [2, 3]), per_ticket_fallback_uses_per_ticket_body_not_truncated_batch (the body substring per-ticket path proves the per-ticket body, not the truncated batch, fed the parser), finish_reason_stop_does_not_trigger_per_ticket_fallback (over-eager-fallback guard), batch_call_4xx_does_not_trigger_per_ticket_fallback (real 401 → exactly 1 LLM call + unanalysed=1 + checks=0 + Notes carries compliance LLM call failed but no per-ticket fallback / falling back annotation — would have failed on r3), batch_call_transport_error_triggers_per_ticket_fallback (real 503 → 1 batch + per-ticket calls in [2, 4] with the fallback mark and the 503 status visible in Notes), and parse_response_accepts_opposite_scheme_work_items_key (the secondary scheme helper, src/compliance/prompt.rs:857). Verification gates: cargo fmt --check clean, cargo clippy --all-targets -- -D warnings clean, cargo test --workspace = 2569 passed / 0 failed / 2 pre-existing #[ignore]; M26's 5 empty-completion tests (empty_completion_triggers_a_disable_thinking_retry_that_recovers / …_with_disable_thinking_already_set_does_not_retry / …_retry_is_bounded_to_one_attempt_when_still_empty / …_retry_only_fires_on_empty_verdicts_not_on_5xx / …_retry_advances_to_next_provider_when_still_empty at src/llm/client/tests.rs:1178/1222/1272/1322/1352) all still green.

Internal

• ExpertScore gained samples_attempted: Option<usize> and samples_failed: Option<usize> (gated by skip_serializing_if = "Option::is_none", so the JSON form samples already implies "sampling was active" — M26 keeps the public contract honest). A Default derive keeps every static-expert / fallback / aggregator / test-fixture initializer untouched via ..Default::default(). M26 counter Arcs (reviewed_chunks, not_reviewed_chunks, not_reviewed_reasons) live in the orchestrator's enclosing scope so the llm_configs.is_empty() local-only path stays a no-op (counters stay zero, summary stays default, renderer's total > 0 gate suppresses the appendix). push_chunk_failure is a small helper that absorbs the #[allow(clippy::expect_used)] so all four chunk-task sites use the same pattern. M32 left parse_status: Vec<ExpertParseStatus>, verification_stats: Option<VerificationStatsSimple>, injected_context: InjectedContext, and confidence_filter_policy: ConfidenceFilterPolicy on ConsolidatedReport (#[serde(default)] for backward compatibility) so M35's wiring setters (with_verification_stats, with_injected_context) could populate them additively. The MAX_SINGLE_FILE_BYTES = 20_000 truncation band is now documented inline; the 20030 / 20047-byte fixture (injected_entry_render_row_handles_injected_greater_than_original) pins the truncated flag as the authoritative status signal rather than the byte ratio.
• Two M17 verification follow-ups (RENG-114 and RENG-115) closed in this release; both were observed on the sirena repro and were out of scope for M17. The two are unrelated in code but appear together in the report because the M17 sirena run surfaced both at once — test_coverage in the breakdown table at 70/100 (RENG-115 symptom) and the four upload.rs mechanical findings the security expert had to absorb (RENG-114 symptom). The cfg_test_line_spans helper lives in experts::facts (where is_test_file already lives) so all three test-detection heuristics share one module. The brace-counting pass in r1 was a string matches('{') / matches('}') per line; r2 replaces that with count_code_braces, a small per-line state machine that tallies braces only outside string / char literals and outside line / block comments, with in_block_comment and in_string threaded across line boundaries. Raw string literals r"…" / r#"…"# and the byte / c-string prefixes are recognised (the #s are counted and the matching "<#×N> is found; inside a raw string the backslash is literal, so a \" does not close the literal — the r1 byte-only approach broke on r#"{\"message\":…}"#). Multi-line strings ("…\" line-continuation) are tracked via the in_string state (the r1 algorithm closed the string at EOL and the next line's { inside {"message":…} was counted as a code brace, producing the same false-negative direction as the P2-2 reviewer counterexample). The state machine is intentionally lightweight (single forward pass, no parser), matching the existing pragmatic line-level scan style of analyse_python_defs in the same module. The r1 doc claim "the only consequence is a span that ends too early, never a swallowed leak" is corrected — a { or } inside a string or comment can extend the span past the real close (false negative: a real leak on the suppressed lines is never scanned); the r2 algorithm prevents both directions in the string / char / comment direction. r3 adds the lifetime rule: a ' is a char literal only when the pattern is '\…or''; otherwise it is a Rust lifetime (&'static, '_, <'a>) and the scanner consumes the '+ following identifier without entering a literal state. Without the rule, lifetimes cause the depth-0 break to fire at an inner} and the span ends early (the reviewer-facts r2 audit caught this on a compiled-harness run — spans ended at (661, 1868) / (672, 770) / (475, 836) / (614, 1203) where the real closes are 1909 / 1549 / 1042 / 1257). The r3 audit (cfg_test_line_spans_does_not_overrun_on_real_source_files) tightens the r2 trim() check to a column-0 }literal so an indented inner}does not pass;cfg_test_line_spans_treats_lifetimes_as_lifetimes_not_char_literalspins the minimal reviewer reproducer.Cargo.lock` is updated for the version bump.
• M15 was the first mission to widen scope into src/actions/repo_review/{scoring,tests,mod}.rs after M14's reservation released. Two follow-up residuals that the author TowerSent earlier ("scoring.rs:56 total_weight_f fallback weights in the divisor"; "full/LLM path's Top Risks bypasses the hygiene filter") are fixed in this release; the report-layer fade-in for hygiene-only experts is also included on the full path (not just local). The reviewer noted the aggregator::aggregate HashMap ordering makes tied Top-Risks runs nondeterministic — pre-existing, not addressed here (the report assembler now uses pick_top_risks on the already-built risk_categories, which sorts stably), called out for the next time the report layer is touched. The aggregator's own AggregatedResult.conclusion.top_risks ("5 lowest merged experts") is intentionally left unchanged so other consumers of that surface are unaffected.
• ExpertScore.reason_code plumbs the schema-failure code end-to-end, and a single-shot clarifying retry recovers non-actionable chunks the M26 disable_thinking retry cannot (M18 self-audit residual, M37 → M38): the post-M30 audit surfaced two chunks (.github/workflows 255 LOC and src/llm 5009 LOC) that landed LLM response not actionable: Code quality assessment completed because the model occasionally returns a non-empty but schema-broken response — the M26 disable_thinking retry inside complete_with_fallback (src/llm/client/mod.rs:643, Empty-gated branches at :735 and :844) only fires when the retry_verdict classifier (src/llm/client/mod.rs:239, returns RetryVerdict::Empty at :243 only on the EmptyCompletion typed marker) hits an empty-content Ok, so the MissingExpectedKeys / UnparseableYaml class bypassed every recovery path. Two coordinated changes close that gap. (i) Single-shot clarifying retry in src/repo/experts/llm_experts/scoring.rs — when parse_expert_yaml returns Null on the single-shot path (scoring.score_samples = 1, the default; the call's median is None so the multi-sample aggregate's safety net doesn't run) and the classifier parse_expert_yaml_diagnosed reports MissingExpectedKeys or UnparseableYaml (NOT EmptyResponse — M26's disable_thinking retry already covers the empty-content class; the two are orthogonal), the call layer sends ONE clarifying follow-up with the same provider chain and the same system prompt plus an appended user-prompt note asking the model to re-emit as the YAML schema specified; upper bound = 2 total attempts on the single-shot path (one clarifying retry max). An Err from the initial call short-circuits via ?, so 5xx never reaches M38. The multi-sample path is unchanged (samples_failed aggregates per-sample drops and the median absorbs one bad sample; a per-sample clarifying retry would inflate the budget to N+1 without benefit). Recovered: ScoringCall.content is the recovered content, reason_code clears to None. Still broken: original content preserved, reason_code carries the initial code and the existing fallback path runs. (ii) reason_code observability — ChunkFailure (src/actions/repo_review/types.rs) gained reason_code: Option<String> with #[serde(default)] so pre-M38 stored JSON deserialises unchanged; the ## Chunks Reviewed table inlines the code inside the Reason cell with a `[reason_code=...]` prefix when present (and leaves the cell unchanged when absent, which preserves the LLM-call-error / M26 empty-completion paths); the JSON contract carries reason_code on each ChunkFailure row (null when absent); and ExpertScore (src/repo/experts/mod.rs) plumbs the code from the LLM expert via a new pub(crate) reason_code: Option<ParseReasonCode> field — the ExpertScoreOutput JSON contract is unchanged. The four ParseReasonCode codes M26 introduced (empty_response / missing_expected_keys / unparseable_yaml / legacy_review_fallback) are reused without adding duplicates. Pinned by 7 unit tests in src/repo/experts/llm_experts/tests.rs — m38_single_shot_no_retry_on_happy_path (clean response → 1 attempt), m38_single_shot_clarifying_retry_recovers_schema_broken_response (broken + valid → 2 attempts, recovered, reason_code == None), m38_single_shot_clarifying_retry_upper_bound_is_one_attempt (broken + broken → 2 attempts, fallback, initial reason code preserved), m38_single_shot_clarifying_retry_records_unparseable_yaml_reason_code (both schema codes round-trip through ScoringCall.reason_code), m38_single_shot_no_retry_on_empty_response_m26_path_unchanged (M26's disable_thinking path intact — exactly 2 samples, M26's own retry only), m38_multi_sample_does_not_fire_clarifying_retry (n=3 multi-sample path unchanged), m38_parse_expert_yaml_diagnosed_classifies_each_failure_path (the classifier the retry keys on) — and 3 tests in src/actions/repo_review/tests.rs — m38_chunk_summary_renders_reason_code_inline (Markdown and JSON both carry `[reason_code=missing_expected_keys]` / `[reason_code=unparseable_yaml]`), m38_chunk_summary_no_reason_code_marker_when_absent (no `[reason_code=` placeholder when the field is None), m38_chunk_failure_deserializes_without_reason_code_field (the #[serde(default)] backward-compat contract on stored JSON without the new field). M26's 5 empty-completion tests (empty_completion_triggers_a_disable_thinking_retry_that_recovers / …_with_disable_thinking_already_set_does_not_retry / …_retry_is_bounded_to_one_attempt_when_still_empty / …_retry_only_fires_on_empty_verdicts_not_on_5xx / …_retry_advances_to_next_provider_when_still_empty) all remain green; gates cargo fmt --check clean, cargo clippy --all-targets --all-features clean, cargo test --lib = 2267 passed / 0 failed / 2 ignored, cargo test --test server = 56 passed, cargo test --test cli = 41 passed.

Notes

• The redaction marker is greppable: every replacement reads ***[redacted:kind=…,len=…,fp=…,key=…]*** (or ***[redacted:kind=…,len=…,fp=…]*** for value-only patterns). An operator can confirm the redaction fired with grep 'redacted:' report.md, and the same fingerprint grep matches the publish-time error message, so the path from "refusal in CI" to "offending line in the local report" is one command.