feat(mt#2926): Force the findings the reviewer's own REQUEST_CHANGES describes

[minsky-ai]

Summary

When the reviewer concludes REQUEST_CHANGES with an empty structured findings channel, the posted
review carries one generic placeholder instead of the issues the model actually described. mt#2828
closed this at the in-loop tool-call boundary, but its own docblock names two paths it cannot reach.
This adds a third post-loop forced pass that covers both.

Diagnosis first, and verified rather than inherited. The task had carried "one of two residual
paths fired — not yet verified" since July. Read from the reviewer service's own structured logs on
the Railway deployment that was live at the time (b6299ad0-…; the current deployment started at
18:39Z, which is why a default railway logs query returns nothing for an 18:20Z review), for
review 5116536812 on PR #3623:

event	field	value
reviewer.empty_findings_recovery_summary	applied	true
	concludeReviewGuardRejectionCount	0
	concludeReviewGuardBoundExhausted	false
reviewer.conclude_review_reminder	mode	post_loop_forced
	fired_at_turn	10
	gate_branch	emitted_no_conclude

rejectionCount: 0 rules out the bound-exhausted path — the in-loop guard never saw a
conclude_review call at all. The loop ran to MAX_TOOL_ROUNDS = 10 without concluding and the
post-loop forced pass supplied the verdict with tool_choice pinned to conclude_review, so
submit_finding was structurally unreachable at the moment the verdict was produced. The whole
output-tool sequence for that review was three calls: submit_spec_verifications (the only one the
main loop produced across ten rounds), then both forced passes.

Magnitude, measured on our own stream rather than on the task's original "2-in-24h burst"
framing: measure-recovery-fire-rate.ts --days=21 over 2026-08-14 → 2026-09-04, 543 PRs, 0 errors —
39 fires across 639 REQUEST_CHANGES rounds (6.1%), 1.8% of all 2113 rounds. That MEETS mt#2828's
registered < 10% budget, so this is a residual rather than a runaway; it is still ~2/day, and each
one costs the consuming agent a manual prose read against a payload shaped exactly like
/implement-task §9's malformed-review bypass condition.

The upstream nudge already ships and did not prevent it. buildRoundBudgetNotice already
injects "Emit any findings you are still holding, then call conclude_review" at two tool-capable
rounds remaining — mt#3547's structural injection, which measurably moved in-loop conclusion 0% →
56% on replay. It produced no finding here, so "tell the model harder" is an exhausted lever and the
remaining fix is a forcing function.

Key changes

• forced-findings-guard.ts (new) — pure trigger predicate, mirroring
  applyEmptyFindingsRecovery's condition exactly (last conclude_review wins, REQUEST_CHANGES,
  zero BLOCKING findings) so what this repairs is precisely what would otherwise reach the mt#2685
  synthesis. Plus the reminder-message builder, which reuses truncateSummaryForDetails so the two
  paths embedding the same unbounded model output cannot drift to different budgets.
• providers.ts — forceFindings(), following forceDocumentationImpact's pattern (full
  ALL_TOOL_DEFINITIONS, pinned tool_choice, shallow-copied messages). Two deliberate differences
  from its siblings: it appends every returned call rather than the first, and it applies the
  mt#2863 / mt#3300 resolution-note guard — without that it would be a second submit_finding
  emission route bypassing an emission guard, which is the shape of gap this whole task exists to
  close.
• Keyed on final accumulated state, not on the gate branch. One predicate therefore covers the
  forced-conclude path and the guard's bound-exhausted fall-through identically — and the fix does
  not depend on the single-review path attribution above generalizing.
• Corrects a stale comment that described mt#1471's narrow tools array long after mt#2722
  replaced it with ALL_TOOL_DEFINITIONS. Its recorded rejected alternative ("retroactive findings
  would be unanchored from evidence the model never gathered") is a claim about the
  emitted_nothing branch; this incident is emitted_no_conclude, where nine rounds of evidence
  were gathered and substantive prose written.
• Retires a can't-fail probe. measure-recovery-fire-rate.ts held its own hard-coded copy of
  the recovery marker sentence, so editing that wording would have made the fire-rate script report
  zero fires with no error — "marker absent" and "pass never fired" are the same observation to it.
  The producer now exports RECOVERY_FIRE_MARKER_PREFIX and builds its text from it; the script
  imports it. This is why SC4's reference to mt#2926 is APPENDED rather than a rewrite of the
  opening sentence.

mt#2685's recovery pass is retained as the backstop and still fires when this pass emits nothing.

Judgment calls

• Scoped out a second occurrence the spec had been carrying with no owner. The 2026-07-29
  PR feat(mt#3299): Mechanical gates wave 1: lint + pre-commit + forced-TZ CI #2392 R4 case (a BLOCKING finding whose text says the issue is resolved) is a different
  mechanism in a different file. Verified against the verbatim finding text that
  RESOLUTION_NOTE_PATTERN does not match it — isResolutionNoteText() returns false on the real
  summary/details pair — and filed as mt#4977. The same round's stale-state re-flagging half is
  mt#4316's class.
• Shipped a partial recovery knowingly. The live smoke shows the pinned pass returns exactly ONE
  finding even when the conclusion names two — a property of the tool_choice primitive, not of the
  wording. Filed as mt#4979 with the measurement. One real located BLOCKING finding still replaces
  one generic placeholder, but SC1 should be read with that ceiling.
• Exported ALL_TOOL_DEFINITIONS so the smoke sends the same tools array production sends;
  rebuilding it in the script would have diverged on exactly the axis mem#614 measured.

Testing

Execution evidence:

AT1 — unit predicate. bun test --preload ../../tests/setup.ts src/forced-findings-guard.test.ts:

 13 pass
 0 fail
 24 expect() calls
Ran 13 tests across 1 file. [87.00ms]

Covers the incident shape (spec verifications + doc impact + REQUEST_CHANGES, zero findings → run),
the bound-exhausted shape, and every skip: BLOCKING finding present, APPROVE, COMMENT, no
conclude_review, empty set, NON-BLOCKING/PRE-EXISTING-only, last-conclude-wins in both directions,
and order-independence.

AT2 — pass mechanics and message non-mutation, and AT3's wiring half. Six cases added to
providers.test.ts driving the real callOpenAIWithClient: fires on the incident shape and lands
two findings at real paths with tool_choice pinned to submit_finding; does not fire (and makes no
extra API call) when a BLOCKING finding is present or on APPROVE; shallow-copies messages; records
the fall-back when the pass returns nothing; applies the resolution-note guard on this path.

AT4 — full reviewer suite, cd services/reviewer && bun run test:

 2520 pass
 0 fail
Ran 2520 tests across 97 files. [4.70s]

Typecheck 0 errors across 8 projects (services/reviewer included, which is where these changes
live); lint 0 errors / 0 warnings across 4385 files.

Negative control — AT2/AT3 wiring. The FULL 65-line forced-findings block was removed from
providers.ts (not a one-line tweak) and providers.test.ts re-run:

(fail) post-loop forced findings pass (mt#2926) > fires on the incident shape and lands the model's own findings in the structured channel
(fail) post-loop forced findings pass (mt#2926) > does not fire when the review already carries a BLOCKING finding — no extra API call
(fail) post-loop forced findings pass (mt#2926) > does not fire on an APPROVE conclusion with zero findings
(fail) post-loop forced findings pass (mt#2926) > shallow-copies messages and ends the forced call with a submit_finding instruction
(fail) post-loop forced findings pass (mt#2926) > records the fall-back when the pass returns no findings, leaving the mt#2685 synthesis to cover it
(fail) post-loop forced findings pass (mt#2926) > applies the mt#2863 resolution-note guard on this path too
 81 pass
 6 fail

6/6 of the new wiring cases fail with the wiring gone. Block restored and re-verified byte-identical.

Negative control — the marker-prefix contract. The opening of the synthesized details was changed
to Recovery pass note (mt#2685): and empty-findings-recovery.test.ts re-run:

(fail) RECOVERY_FIRE_MARKER_PREFIX > the synthesized finding's details still START with the marker prefix
 13 pass
 1 fail

The pin catches exactly the edit that would silently blind the fire-rate script. Reverted and
re-verified.

[at5-deferred: mt#4980] — AT5 is a post-deploy fire-rate measurement over a review-volume window;
mt#4980 owns it and carries the 6.1% pre-change baseline to compare against.

Live verification

services/reviewer/scripts/smoke-forced-findings.ts (new, §7a artifact, OPENAI_API_KEY-gated,
exits 0/2, writes a structured results file), run against live gpt-5 with the production
ALL_TOOL_DEFINITIONS array and the real reminder builder, on a conclusion naming two file-level

=== mt#2926 forced-findings live smoke (gpt-5, 3 attempts) ===
attempt 1: PASS — 1 finding(s) parsed from 1 tool call(s)
    BLOCKING src/hooks/pre-commit.ts:1
attempt 2: PASS — 1 finding(s) parsed from 1 tool call(s)
    BLOCKING src/hooks/pre-commit.ts:1
attempt 3: PASS — 1 finding(s) parsed from 1 tool call(s)
    BLOCKING src/hooks/pre-commit.ts:1

=== Result: 3/3 attempts emitted >= 1 parseable finding ===

3/3 emitted a parseable BLOCKING finding at a real repository path, not the mt#2685
(review summary) sentinel. That is the model-side property no unit test can reach.

Two bounds, stated rather than implied:

1. One finding per attempt, though the conclusion named two — the pinned tool_choice
   primitive, which providers.ts already documents as forcing exactly one call. Filed as mt#4979.
2. line: 1 is an artifact of the fixture, not a production finding. The smoke gives the model
   no diff and no read_file results, so it has nothing to anchor a line to. It measures compliance
   with the pin and the shape of the returned call; it does not measure line-number accuracy.

The smoke calls the API directly rather than driving callOpenAIWithClient, so it exercises the
model's half only — the six wiring cases above cover the service's half, with the negative control
that proves they can fail.

Deploy verification

Deploy verification: every changed file is deploy surface (confirmed by running
isDeploySurfaceFile from packages/domain/src/deployment/deploy-surface.ts over the actual
changed-file list — all five source paths return true), so no [no-deploy-impact] claim is made.
After merge I will run deployment_wait-for-latest for minsky-reviewer with notBefore set to
the merge timestamp and expectCommitSha set to the merge commit, read buildIdentity, and assert
the /health body's service identity rather than the status code. This change adds no new
external-system integration — no new permission, scope, credential, or webhook; the new pass is one
more call to an already-configured provider on an already-provisioned key — so §10's
external-integration live-exercise requirement does not apply, and the deploy-health check plus the
first production reviewer.forced_findings_pass log line is the completion signal.

🤖 Generated with Claude Code

https://claude.ai/code/session_012HYHcmDv7NuaD6uK7uAvU2

[minsky-reviewer]

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 373K prompt, 5K completion | Duration: 83s
Mode: normal

Commands

• /review — request a fresh review

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

One blocking issue found: the reviewer.forced_findings_pass log incorrectly sets fired: true even when no forced submit_finding call can be attempted (missing tool definition), inflating the pass’s measured fire rate. Please propagate an attempted flag from forceFindings (or emit the event within it) and set fired accordingly. After this fix, the PR appears ready to merge.

Findings

• [BLOCKING] services/reviewer/src/providers.ts:1989 — reviewer.forced_findings_pass logs fired: true even when no API call can run (SUBMIT_FINDING_TOOL_DEF missing)
  In forceFindings() when SUBMIT_FINDING_TOOL_DEF is null, the function returns early with emittedCount: 0 and no API call is made (see services/reviewer/src/providers.ts:1190-1210). However, the call site unconditionally logs reviewer.forced_findings_pass with fired: true (services/reviewer/src/providers.ts:1989-2004) regardless of whether a forced call actually executed. This over-reports fires and corrupts post-deploy measurements (SC5) by counting disabled/misconfigured paths as successful "fires". Fix: propagate a flag from forceFindings indicating whether a provider call was attempted (e.g., attempted: boolean), and set fired accordingly; or move the fired log into forceFindings to centralize truth. Until then, observability is misleading.

Spec verification

Criterion	Status	Evidence
The forced path can no longer post a REQUEST_CHANGES with an empty findings channel — after post-loop passes, if the final accumulated conclusion is REQUEST_CHANGES and no BLOCKING submit_finding exists, run one additional tool_choice-pinned submit_finding pass and append every returned call.	Met	Trigger/pin/wiring: services/reviewer/src/forced-findings-guard.ts:89-119 (predicate mirrors recovery); services/reviewer/src/providers.ts:1967-2007 (unconditional evaluation keyed on final state) and 1219-1293 (forceFindings pins tool_choice to submit_finding and appends all parseable calls). Backstop mt#2685 retained at services/reviewer/src/empty-findings-recovery.ts:118-170 to ensure the channel is still non-empty even when the forced pass yields nothing.
Keyed on final accumulated state, not on gate branch — the predicate must fire for both the post-loop forced-conclude path and the guard’s bound-exhausted fall-through.	Met	The decision function reads only accumulated tool calls and applies the "last conclude_review wins" rule (services/reviewer/src/forced-findings-guard.ts:65-88), with no reference to gate-branch. The caller invokes it after both post-loop passes (services/reviewer/src/providers.ts:1967-1979).
Bounded and non-fabricating — at most one extra API call per review; never triggers on APPROVE/COMMENT nor when a BLOCKING finding is already present.	Met	Single invocation site after loop (one pass max) at services/reviewer/src/providers.ts:1967-2007; guard returns skip for APPROVE/COMMENT or existing BLOCKING at services/reviewer/src/forced-findings-guard.ts:98-119. No loop or retries around the forced findings call.
mt#2685’s recovery pass remains as backstop and its synthesized finding’s text appends the mt#2926 reference without changing the marker prefix used by the measurement script.	Met	RECOVERY_FIRE_MARKER_PREFIX exported and used to build details (services/reviewer/src/empty-findings-recovery.ts:38-57, 118-170). The mt#2926 note is appended after the unchanged prefix (same lines). The measurement script now IMPORTS the prefix (services/reviewer/scripts/measure-recovery-fire-rate.ts:32-42,53-61). A test pins the prefix-at-start invariant (services/reviewer/src/empty-findings-recovery.test.ts:204-236).
Observability — a structured reviewer.* log records the new pass’s outcome (fired / findings emitted / failed) to re-measure post-deploy.	Met	Logging on both fire and skip paths at services/reviewer/src/providers.ts:1989-2004 (success/empty) and 2005-2018 (error), plus unconditional skip record at 2019-2028. Note: see separate blocking finding about the fired flag being set true even when no provider call could be attempted (SUBMIT_FINDING_TOOL_DEF missing).
The stale forced-conclude tool-list comment is corrected to reflect full ALL_TOOL_DEFINITIONS + tool_choice pin semantics.	Met	Comment updated at services/reviewer/src/providers.ts:1887-1913 explaining mt#2722’s widened tools array with conclude_review pin, replacing the prior stale description.

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
services/reviewer/src/forced-findings-guard.evaluateForcedFindingsPass	function	services/reviewer/src/providers.ts:1967 — evaluates trigger against accumulated tool calls, services/reviewer/src/forced-findings-guard.test.ts:37 — unit tests cover predicate	Adopted

Documentation impact

• no-update-needed — This PR changes internal reviewer-service behavior and test scripts. No user-facing CLI, API, or documented workflow semantics changed beyond internal observability and recovery mechanisms. The measurement script was updated to import a constant; no docs reference needed updating. I checked services/reviewer/README.md (unchanged per diff) and found no narrative docs covering the forced-findings mechanism.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Implements a coherent forced-findings pass with solid tests and observability; success criteria are met. However, one blocking design gap remains: the forced pass appends every parsed submit_finding from a single pinned call without any deduplication or ceiling, enabling amplification or duplicate BLOCKING findings from one forced request (providers.ts:1967-2016). Add deduplication against existing and intra-response duplicates and/or a strict cap (or first-call-only) to align with the bounded, non-fabricating intent. Non-blocking notes: (1) smoke script imports ALL_TOOL_DEFINITIONS from providers, coupling to heavy deps; consider a lighter export; (2) the user message’s “do not invent a location” conflicts with the required line field — consider a sanctioned fallback or field for unlocatable issues. With the dedupe/cap fix, this is ready to merge.

Findings

• [BLOCKING] services/reviewer/src/providers.ts:1967 — Forced findings pass can fabricate multiple findings from a single pinned call without bounding or deduplication
  At providers.ts:1967-2016 the forced findings path appends "every" parsed submit_finding tool call from a single pinned tool_choice request. While the text elsewhere assumes OpenAI typically returns one, the code allows N calls in the same response. This introduces two risks: (1) duplication of the same issue (model echoes identical calls) and (2) manufacturing multiple blocking findings from a single forced pass that was intended to "structure the verdict" rather than expand it. There is no deduplication against existing accumulatedToolCalls, no cap beyond the single API call, and no guard against identical file/line/summary tuples. To keep the mechanism bounded and non-fabricating per the spec intent, cap appended findings to at most one per unique (file,line) pair, or at minimum perform a deduplication check against existing calls and within the response. Alternatively, constrain to the first valid call (matching the pattern used by forceConcludeReview/forceDocumentationImpact) and document the limitation, or batch with a strict N ceiling (e.g., 2) to prevent amplification on a forced path.
• [NON-BLOCKING] services/reviewer/scripts/smoke-forced-findings.ts:86 — Importing ALL_TOOL_DEFINITIONS from src/providers couples the smoke to a heavy module
  The smoke script imports ALL_TOOL_DEFINITIONS from ../src/providers (line 86). That module brings in the full provider stack (OpenAI SDK, logger, retry helpers, etc.) at module-eval time. While there are no obvious side effects, this increases startup cost and risks incidental coupling to provider-internal changes. Consider exporting the tool definitions from a lighter-weight module (e.g., re-export OUTPUT_TOOL_DEFINITIONS via a dedicated src/tools-registry.ts) or importing directly from src/output-tools and mapping to the OpenAI SDK shape locally in the script. This keeps the smoke’s dependency surface minimal and avoids accidental provider-coupled failures.
• [NON-BLOCKING] services/reviewer/src/forced-findings-guard.ts:161 — User message tells model not to invent a location but still requires a line number; no designed observable for truly unlocatable issues
  buildForcedFindingsUserMessage() (around line 161) instructs the model to "anchor each finding to a file and line you actually read" and "do NOT invent a location", then suggests anchoring to a file and saying so in details if not locatable. However, SubmitFindingArgsSchema requires line and the forced pass pins tool_choice, so the model has no way to represent a non-line-locatable blocking issue without making up a line number. Consider adding a designed observable for unlocatable-but-real blockers in this path (e.g., allow line: 1 with side omitted and a standardized "unlocatable" flag in details, or a separate tool schema/field acknowledged by downstream composition). Absent that, the instruction is contradictory and can pressure fabricated line numbers.

Spec verification

Criterion	Status	Evidence
The forced path can no longer post a REQUEST_CHANGES with an empty findings channel. After the post-loop forced passes complete, when the accumulated conclude_review is event: "REQUEST_CHANGES" and no BLOCKING submit_finding has been accumulated, the service runs ONE additional tool_choice-pinned submit_finding pass and appends every returned call.	Met	services/reviewer/src/providers.ts:1967-2016 — new evaluateForcedFindingsPass() gate and forceFindings() call; services/reviewer/src/forced-findings-guard.ts:130-186 — trigger predicate; services/reviewer/src/providers.test.ts:2710-3009 — test “fires on the incident shape and lands the model's own findings in the structured channel.”
Keyed on final accumulated state, not on the gate branch, so it covers both the forced-conclude path and the guard’s bound-exhausted fall-through.	Met	services/reviewer/src/forced-findings-guard.ts:130-186 — selects last conclude_review and checks BLOCKING count over accumulatedToolCalls; services/reviewer/src/providers.ts:1948-2016 — pass runs unconditionally after both forced passes based on accumulated state.
Bounded and non-fabricating: at most one extra API call per review, and only when event=REQUEST_CHANGES with zero BLOCKING findings; APPROVE/COMMENT or reviews already carrying a BLOCKING finding never trigger it.	Met	services/reviewer/src/providers.ts:1948-2016 — single forceFindings API call guarded by evaluateForcedFindingsPass; services/reviewer/src/forced-findings-guard.test.ts:58-147 — skip cases for APPROVE, COMMENT, and presence of BLOCKING findings.
mt#2685’s recovery pass is retained as the backstop and still fires when this pass emits nothing; the synthesized finding text appends mt#2926 reference without changing the marker prefix used by the measurement script.	Met	services/reviewer/src/empty-findings-recovery.ts:72-101,153-181 — exports RECOVERY_FIRE_MARKER_PREFIX and builds details starting with it; services/reviewer/src/empty-findings-recovery.test.ts:11-36 — test pins prefix; services/reviewer/scripts/measure-recovery-fire-rate.ts:41-51 — now imports the prefix; providers.ts:1997-2016 logs fell_back_to_recovery_synth when none parsed.
Observability: a structured reviewer.* log event records the new pass’s outcome (fired / findings emitted / failed).	Met	services/reviewer/src/providers.ts:1997-2016 — emits reviewer.forced_findings_pass with fired, emitted_count, fell_back_to_recovery_synth and error on failure; providers.test.ts:2710-3009 asserts log presence via findLogEvent.
The stale forced-conclude tool-list comment is corrected to reflect ALL_TOOL_DEFINITIONS + tool_choice pin.	Met	services/reviewer/src/providers.ts:1713-1758 — corrected comment explains full tools array with pinned tool_choice (mt#2722), replacing outdated narrow-array description.

Documentation impact

• no-update-needed — This PR changes internal reviewer-service behavior and test/smoke scripts. No user-facing CLI, API, or documented configuration changed. The only doc-relevant constant (RECOVERY_FIRE_MARKER_PREFIX) is explicitly exported and tests pin that posted-body text still starts with the same marker; the measurement script was updated to import it. I checked docs/ for reviewer-service user docs and found no references that would be invalidated by these internal mechanics.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Re-verified the fixes since the last round. The BLOCKING concern about miscounting the forced-findings pass as “fired” without attempting a provider call is addressed: describeForcedFindingsOutcome now distinguishes not-attempted vs attempted outcomes, and providers.ts uses it when logging reviewer.forced_findings_pass. The forced-findings mechanism remains correctly keyed on final accumulated state, is bounded (one extra call only on incoherent REQUEST_CHANGES), appends all returned findings, and retains the mt#2685 backstop. Tests cover the predicate, message shape, and logging semantics. I found no new critical defects introduced by this fix. Verdict: APPROVE.

Spec verification

Criterion	Status	Evidence
The forced path can no longer post a REQUEST_CHANGES with an empty findings channel. After the post-loop forced passes complete, when the accumulated conclude_review is REQUEST_CHANGES and no BLOCKING submit_finding has been accumulated, the service runs ONE additional tool_choice-pinned submit_finding pass whose injected user message carries the conclusion summary and instructs the model to emit a structured finding for each blocking issue it just named. Every submit_finding call that pass returns is appended to accumulatedToolCalls before composition.	Met	services/reviewer/src/providers.ts:1974-2046 — wires a new post-loop forced-findings call, pinning tool_choice to submit_finding and appending every parsed call to accumulatedToolCalls; services/reviewer/src/forced-findings-guard.ts:124-158 — trigger predicate mirrors the empty-findings condition and returns the conclusion summary for the user message; services/reviewer/src/forced-findings-guard.test.ts:20-118 — unit asserts the incident shape triggers and BLOCKING-present shape skips.
Keyed on final accumulated state, not on the gate branch.	Met	services/reviewer/src/forced-findings-guard.ts:100-122 — evaluates over final accumulatedToolCalls and uses the last conclude_review; services/reviewer/src/providers.ts:1974-1989 — forced-findings evaluation runs unconditionally after the forced-conclude pass; services/reviewer/src/forced-findings-guard.test.ts:41-61 — tests both post-loop-forced and bound-exhausted shapes.
Bounded and non-fabricating. At most one extra API call per review, and only on the incoherent shape: APPROVE and COMMENT conclusions, and any REQUEST_CHANGES that already carries a BLOCKING finding, never trigger it.	Met	services/reviewer/src/providers.ts:1990-2046 — executes a single withTimeout-wrapped API call when evaluateForcedFindingsPass returns decision "run"; services/reviewer/src/forced-findings-guard.ts:134-158 — skips on APPROVE/COMMENT or BLOCKING-present; services/reviewer/src/forced-findings-guard.test.ts:88-116, 65-86 — pins skip cases.
mt#2685's recovery pass is retained as the backstop and still fires when the new pass emits nothing (API error, parse error, or a response carrying no submit_finding). Its synthesized finding's text references this task alongside mt#2685 while it still fires. The reference is APPENDED, never a rewrite of the opening sentence: services/reviewer/scripts/measure-recovery-fire-rate.ts:51 hard-codes the literal "Synthesized by the empty-findings coherence recovery pass (mt#2685)" as its own copy of the marker (it is not a shared constant), so altering that prefix makes the measurement script report zero fires with no error — the exact can't-fail-probe shape. A test pins the marker prefix, and SYNTHESIZED_FINDING_FILE = "(review summary)" is left untouched because review-provenance.ts:100 counts synthesizedBlocking off it.	Met	services/reviewer/src/providers.ts:2031-2046 — logs and falls through; composition remains unchanged so applyEmptyFindingsRecovery still runs downstream; services/reviewer/src/empty-findings-recovery.ts:79-119, 152-199 — RECOVERY_FIRE_MARKER_PREFIX retained and mt#2926 reference appended (opening unchanged); services/reviewer/src/forced-findings-guard.test.ts:183-256 — pins describeForcedFindingsOutcome so API errors/empty results are distinguished and counted as fallback.
Observability. A structured reviewer.* log event records the new pass's outcome (fired / findings emitted / failed) on the same stream as reviewer.empty_findings_recovery_summary, so the 6.1% baseline above can be re-measured post-deploy without new machinery.	Met	services/reviewer/src/providers.ts:2010-2046 — emits reviewer.forced_findings_pass with fields fired, fired_at_turn, emitted_count, fell_back_to_recovery_synth and skip_reason/error; services/reviewer/src/forced-findings-guard.ts:164-252 — helper maps outcomes to log fields to avoid miscounting fired.
The stale forced-conclude tool-list comment is corrected to describe what mt#2722 actually ships (full ALL_TOOL_DEFINITIONS plus the tool_choice pin), with its rejected-alternative note re-stated against the emitted_nothing branch it actually applies to.	Met	services/reviewer/src/providers.ts:1734-1764 — comment updated to state full ALL_TOOL_DEFINITIONS with tool_choice pin and to clarify why retroactive findings were rejected only for the emitted_nothing branch.

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
ALL_TOOL_DEFINITIONS	capability	services/reviewer/scripts/smoke-forced-findings.ts:18 — imports the array to mirror production tools list, services/reviewer/src/providers.ts — defines and uses locally for forced passes	Adopted	Export added specifically for the live smoke script; already consumed there.

Documentation impact

• no-update-needed — The changes add an internal post-loop forced-findings mechanism and adjust service logging. No user-facing commands, APIs, or documented behavior were added or changed. Docs about the recovery pass marker remain accurate (prefix unchanged).

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Verified the fix commit addresses the prior blocking concern about miscounting the forced-findings pass as “fired” without an actual provider attempt. The new describeForcedFindingsOutcome and its use in providers.ts correctly distinguish not-attempted vs attempted/failed outcomes, and tests assert the logging semantics. The PR implements the post-loop forced-findings pass per spec: keyed on final accumulated state, bounded to one extra call only on incoherent REQUEST_CHANGES, appends all returned findings, retains the mt#2685 backstop, corrects the stale comment, and improves observability and the measurement script via an exported marker prefix. I find no new critical defects introduced by these changes. Verdict: APPROVE.

Spec verification

Criterion	Status	Evidence
The forced path can no longer post a REQUEST_CHANGES with an empty findings channel. After the post-loop forced passes complete, when the accumulated conclude_review is REQUEST_CHANGES and no BLOCKING submit_finding exists, run one tool_choice-pinned submit_finding pass and append every returned call.	Met	services/reviewer/src/providers.ts:1974-2059 — calls evaluateForcedFindingsPass then forceFindings(...) and appends parsed submit_finding calls to accumulatedToolCalls; services/reviewer/src/forced-findings-guard.ts:75-123 — predicate requires REQUEST_CHANGES with zero BLOCKING findings; services/reviewer/src/providers.test.ts:2715-2897 — test asserts two structured findings are appended and real files returned.
Keyed on final accumulated state, not on the gate branch (covers both post-loop forced conclude and guard bound-exhaust).	Met	services/reviewer/src/forced-findings-guard.ts:75-123 — scans the final accumulatedToolCalls, takes the LAST conclude_review, and checks for any BLOCKING finding; services/reviewer/src/forced-findings-guard.test.ts:29-111 — tests both incident shape and bound-exhausted shape; services/reviewer/src/providers.ts:1974-1996 — unconditional evaluation after both forced passes.
Bounded and non-fabricating: at most one extra API call; never triggers on APPROVE/COMMENT and not when a BLOCKING finding is already present.	Met	services/reviewer/src/providers.ts:1997-2059 — single forceFindings call only when predicate says run; services/reviewer/src/forced-findings-guard.ts:101-123 — skips for APPROVE/COMMENT and when any BLOCKING finding exists; services/reviewer/src/forced-findings-guard.test.ts:113-180 — tests skip cases; services/reviewer/src/providers.test.ts:2899-2971 — no extra API call when a BLOCKING finding exists; 2973-3009 — skips on APPROVE.
mt#2685 recovery pass retained as backstop; synthesized finding text references mt#2926 by appending, not rewriting the opening marker (measurement import).	Met	services/reviewer/src/empty-findings-recovery.ts:153-182 — details now ${RECOVERY_FIRE_MARKER_PREFIX}: … mt#2926 added … (appended after marker); services/reviewer/scripts/measure-recovery-fire-rate.ts:41-57 — now imports RECOVERY_FIRE_MARKER_PREFIX instead of hardcoding; services/reviewer/src/empty-findings-recovery.test.ts:197-232 — pins that details START with the marker prefix.
Observability: structured reviewer.* log event records fired / findings emitted / fallback vs skip.	Met	services/reviewer/src/providers.ts:2009-2059 — emits reviewer.forced_findings_pass with fired, emitted_count, fell_back_to_recovery_synth, and skip_reason/error; services/reviewer/src/forced-findings-guard.ts:133-213 — describeForcedFindingsOutcome defines the mapping; services/reviewer/src/providers.test.ts:2715-2897, 3011-3056 — asserts log fields for fired/skip/fallback.
Correct the stale forced-conclude tool-list comment to reflect full ALL_TOOL_DEFINITIONS with tool_choice pin, and restate the rejected alternative correctly.	Met	services/reviewer/src/providers.ts:1713-1732 — updated comment explains full tools array plus pin (mt#2722), and clarifies the earlier rejected alternative’s scope.

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
services/reviewer/src/providers.ts — ALL_TOOL_DEFINITIONS	function	services/reviewer/src/providers.ts — internal uses in forceConcludeReview/forceDocumentationImpact/forceFindings, services/reviewer/scripts/smoke-forced-findings.ts: imports ALL_TOOL_DEFINITIONS for live smoke	Adopted	Export added to allow the smoke script to use the exact same tool list as production (per mt#2926).

Documentation impact

• no-update-needed — Internal reliability/mechanics change to reviewer service; no user-facing commands, APIs, or documented workflows were added or changed. The diff updates logging, a post-loop forced findings pass, and a measurement script. No docs in repo reference these internals, and no existing user-facing behavior descriptions are invalidated.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Verification round: the prior BLOCKING concern (miscounting the forced-findings pass as “fired” without an actual provider attempt) is addressed via describeForcedFindingsOutcome and corrected logging in providers.ts. The new forced-findings pass is correctly keyed on final accumulated state, bounded to one extra call only on incoherent REQUEST_CHANGES, appends all returned findings, applies the resolution-note guard, and retains the mt#2685 backstop. Tests cover predicate, wiring, message shape, and logging; the marker-prefix export removes a can’t-fail probe. I found no new critical defects introduced by these changes. Verdict: APPROVE.

Spec verification

Criterion	Status	Evidence
The forced path can no longer post a REQUEST_CHANGES with an empty findings channel. After the post-loop forced passes complete, when the accumulated conclude_review is REQUEST_CHANGES and there are zero BLOCKING submit_finding calls, run ONE additional tool_choice‑pinned submit_finding pass that instructs the model to emit a structured finding for each blocking issue named; append every returned call to accumulatedToolCalls.	Met	services/reviewer/src/providers.ts:1974-2038 — invokes the new forced-findings pass unconditionally after forced conclude, keyed on final accumulated state; services/reviewer/src/forced-findings-guard.ts:133-168 — predicate matches the empty-BLOCKING condition and returns the authoritative conclusion summary; services/reviewer/src/providers.ts:1129-1236 — forceFindings() pins tool_choice to submit_finding and appends every parseable returned call; tests at services/reviewer/src/providers.test.ts:2712-2875 assert the pass fires on the incident shape and appends findings.
Keyed on final accumulated state, not on the gate branch (covers both post-loop forced-conclude and in-loop bound‑exhausted fall-through). A unit test pins both.	Met	services/reviewer/src/forced-findings-guard.ts:95-131 — scans all accumulated tool calls, applies 'last conclude_review wins', and checks for absence of BLOCKING findings; services/reviewer/src/forced-findings-guard.test.ts:48-85 — tests the incident shape and the bound‑exhausted shape; services/reviewer/src/providers.ts:1974-1991 — evaluates the predicate once after all forced passes.
Bounded and non‑fabricating: at most one extra API call per review, and only when conclude=REQUEST_CHANGES with zero BLOCKING findings; APPROVE/COMMENT or already‑BLOCKING never trigger it.	Met	services/reviewer/src/providers.ts:1974-2038 — single conditional call site; services/reviewer/src/forced-findings-guard.ts:117-168 — skips on APPROVE/COMMENT, on no conclude, and when any BLOCKING finding already exists; services/reviewer/src/providers.test.ts:2820-2875 — tests assert no extra API call for APPROVE and for existing BLOCKING finding and check skip_reason logging.
mt#2685’s recovery pass is retained as the backstop and still fires when the new pass emits nothing; its synthesized finding text now APPENDS an mt#2926 reference without changing the opening marker used by the measurement script.	Met	services/reviewer/src/empty-findings-recovery.ts:153-185 — details now prepend RECOVERY_FIRE_MARKER_PREFIX unchanged and append explanatory mt#2926 note; services/reviewer/src/empty-findings-recovery.test.ts:197-233 — pins that details START with RECOVERY_FIRE_MARKER_PREFIX; services/reviewer/scripts/measure-recovery-fire-rate.ts:41-55 — now imports RECOVERY_FIRE_MARKER_PREFIX instead of hard‑coding; services/reviewer/src/providers.test.ts:2860-2875 — verifies fallback path logging when forced pass emits nothing.
Observability: a structured reviewer.* event records the new pass’s outcome (fired / findings emitted / failed) on the same stream as reviewer.empty_findings_recovery, so post‑deploy rate can be measured.	Met	services/reviewer/src/providers.ts:2004-2038 — logs reviewer.forced_findings_pass with fired, emitted_count, fell_back_to_recovery_synth, skip_reason/error; services/reviewer/src/forced-findings-guard.ts:171-249 — describeForcedFindingsOutcome encodes log semantics; services/reviewer/src/forced-findings-guard.test.ts:179-244 — unit tests cover not‑attempted/attempted/failed mappings; services/reviewer/src/providers.test.ts:2755-2816 — asserts event presence and fields.
The stale forced‑conclude tool‑list comment is corrected to describe the actual shipped behavior (full tools array with tool_choice pin), and the rejected‑alternative note is scoped to the emitted_nothing branch it applies to.	Met	services/reviewer/src/providers.ts:1711-1731 — updated comment explains ALL_TOOL_DEFINITIONS with tool_choice pin and clarifies the earlier rejected alternative’s scope per mt#2926.

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
services/reviewer/src/providers.ALL_TOOL_DEFINITIONS	function	services/reviewer/scripts/smoke-forced-findings.ts: imports ALL_TOOL_DEFINITIONS for live smoke, services/reviewer/src/providers.ts: defined and used at multiple forced passes (OpenAI calls)	Adopted	Exported to ensure the smoke script uses the exact same tools array as production per mt#2926.
services/reviewer/src/empty-findings-recovery.RECOVERY_FIRE_MARKER_PREFIX	function	services/reviewer/scripts/measure-recovery-fire-rate.ts:49 — imports RECOVERY_FIRE_MARKER_PREFIX to avoid hard-coding the marker	Adopted	Exported to eliminate a duplicated literal; the measurement script now imports it per mt#2926.

Documentation impact

• no-update-needed — Internal reviewer-service reliability fix: adds a post-loop forced-findings pass, logging, and exports a marker constant. No public API, CLI, or user-facing documented behavior changed. Scripts and tests were updated; no docs reference these internals. Checked services/reviewer docs tree implicitly via code paths; no invalidated prose identified.