fix(review): the four deferred #42 findings — causal finding attribution and a member-aware v2 lifecycle

[stranske]

The four findings from the CodeRabbit review of #42 that were deliberately deferred because each one changes learning-loop semantics, where CLAUDE.md §2 says a wrong call corrupts the Brain quietly rather than failing loudly.

They were fixed and pushed to claude/profile-per-agent at 3f43569, but #42 had already merged at 9239e06b twelve minutes earlier, so that commit never reached main. This is the same commit cherry-picked onto af6654d6 — clean, and nothing else on main touched those files after #42 landed. The three inline threads on #42 carry the full per-finding replies; #42 also carries a note that they did not ship with the merge.

---

1. Durability inferred from an arbitrary run — research_subjects.py (thread)

resolve_round_durability picked an issue's outcome with ORDER BY r.ts DESC LIMIT 1 over every run that had ever targeted it, so a later review, audit or unrelated run silently replaced the delivery outcome and rewrote both per_arm_durability and the accepted influence edge.

The delivering run cannot be known at filing time — the issue is filed before anything implements it — so this is a schema change, not a query tweak:

• finding_implemented, a new decision in the same research_subject_events log (no new table, so no migration), written by record_finding_implementation(). Brain-validated: the run must exist and its recorded target must be that issue. The arm is inherited from the filing, never taken as an argument, so a binding cannot re-attribute a finding.
• No heuristic replaced the sort. Absent a binding, resolution requires exactly one scored run; two or more stays unresolved as ambiguous_outcome_runs. Newest / first / "looks like an implement" would each let an unrelated run rewrite an arm's durability, which is the defect.
• The gap reports its own drain so it cannot latch quietly: unresolved_by_reason splits the causes, drainable_by_binding counts the only one that drains through nothing on its own (no_outcome_run drains when work lands, bound_run_has_no_outcome when the run is scored), and drain names the command.

2. Unvalidated arm attribution — research_subjects.py (thread)

record_finding_issue stored a caller-supplied arm, so a finding could be attributed to another participating agent and resolve_round_durability would then build an accepted edge from that agent's run. Membership now comes from round_arm_evidence() — the round's registered arm set, or a run bound to the round by experiment_id. A round with neither is refused outright.

The rule already existed at a second seam (completion_event_adapter's selected_arm_not_in_subject_set), so it is centralised in one reader rather than hand-rolled a third time — two seams disagreeing about who was in a round is exactly the drift that matters, because the learner treats each arm as independent evidence.

3. Arms registered for agents that never ran — partitioned_review.py (review body, reported outside the diff)

register_review_round recorded every --round-agents entry as an arm while run_plan dispatches every partition to --agent alone, so --agent gemini --round-agents gemini,cursor registered two arms of comparative evidence one of which never ran.

Neither option the reviewer offered was right: restricting the set to the executing agent breaks the real multi-seat case (--round-agents exists so several run invocations land on one subject). So an arm is admitted only on a record — the executing seat, a round-bound run, or a prior registration, meaning the set can only grow. That last clause is load-bearing: finding 2 validates against this set, so a resumed single-seat run that shrank it would start refusing the other seats' real findings. Declared arms with no record are dropped and named in arms_unproven.

4. v2 experiment lifecycle not member-aware — exp_abcd.py, experiment_recovery.py (thread)

followup measured eligibility per AGENT (<agent>.log from meta["agents"]) while prepare_arms writes one log per MEMBER, so no v2 experiment ever reached collect/evaluate and the v2 identity work produced zero evaluations_v2 rows — eaf2f7b half-fixed. Logs now come from experiment_members + exp_log_path, which keeps the legacy shape working because a legacy member's id is its agent (that is what recovered directories write).

collect()'s branch recovery had the matching gap — v2 branches are exp/<exp_id>-<member_id> — so it asked for a branch that cannot exist, read that as "the evidence is gone", and followup stamped followup-skip.json while the commits sat intact in the shared store. arm_diff now takes a keyword-only member_id.

---

Test gate

Three new tests plus selftest coverage, each with a deliberate-break → revert demonstration actually run:

Break	Result
restore newest-run-wins	selftest fails: per_arm_durability: {'codex': {'reverted': 1}} — the mis-attribution, reproduced
trust the caller's arm	selftest fails: arm outside the round's set accepted: nobody
believe the --round-agents declaration	2 pytest failures
list meta["agents"] logs	['legacy-exp'] == ['legacy-exp', 'v2-exp'] — legacy passes, v2 skipped: the half-fixed state exactly
drop member_id from arm_diff	branch recovery misses the member branch

• test_partitioned_review.test_multi_agent_arm_set_is_admitted_only_on_a_round_bound_attempt — covers all three reviewer cases (dropped / correctly scheduled / never shrinking), then files from both real seats and asserts a third agent gets arm_not_in_subject_set
• test_experiment_arm_identity.test_followup_collects_and_evaluates_a_v2_manifest — builds the manifest through the production launcher path (research_v2_arms → _normalize_arm_members), asserts the member log names really are disjoint from the agent names, drives followup to collect+evaluate both a v2 and a legacy experiment, and checks idempotency
• test_experiment_arm_identity.test_branch_recovery_resolves_the_v2_member_branch — stubs _git rather than arm_diff, so the real branch name is composed and asserted
• research_subjects --selftest — both refusals (unknown arm; a real agent that is an arm of a different round), the ambiguity → binding → resolution path, and binding validation (unknown run, run that targeted a different issue, binding with no filing)

The existing round test was asserting the buggy behaviour (arms == ["gemini","cursor","codex"] from a gemini-only dispatch) and now asserts the fix.

Verification

python3 verify.py green, and run twice — once here and once from a mirror-shaped tree, per the rule that cmp-clean is not agreement. The live ~/.codex/orchestrator-mirror was deliberately not synced: ORCH_MIRROR was pointed at a scratch path, so the layout check runs while the dispatcher keeps running merged code and CLAUDE.md §1's manual circuit breaker stays intact.

Floor re-measured on this base rather than carried over from the pre-merge branch. Dedup findings recorded in the capability ledger notes for abcd-experiment and partitioned-review per CLAUDE.md §0 — in both cases the concept already existed and this wired or centralised it rather than building anything new.

🤖 Generated with Claude Code

---

Scope

Only the four findings from the #42 CodeRabbit review that were deliberately deferred there because each changes learning-loop semantics. Touches research_subjects.py, partitioned_review.py, exp_abcd.py, experiment_recovery.py, their two test files, and .verify-floor.json. No new table, no migration, no new event log, no flag flipped, no capability added.

Tasks

• Replace the ORDER BY r.ts DESC LIMIT 1 durability inference with a persisted, Brain-validated finding_implemented binding; leave ambiguous targets unresolved and report the drain
• Validate the filing arm against the round's registered/attempted arm set before writing FINDING_FILED
• Admit review-round arms only on a record (executing seat, round-bound run, or prior registration); name dropped declarations in arms_unproven
• Derive followup eligibility from experiment_members + exp_log_path, and pass member_id through branch recovery
• One test per fix, each with a deliberate-break → revert demonstration actually run
• Re-measure .verify-floor.json on the merge result (405), union-resolving the three conflicts rather than taking a side

Acceptance criteria

• verify.py green in this checkout: 405 passed, 0 failed, 0 skipped; 84/84 selftests; 5/5 gates
• verify.py green in the exec-mirror layout (the cmp-clean-is-not-agreement second run): 394 passed + 11 named skips = 405 collected; 84/84 selftests; 5/5 gates
• verify.py green on CI: 379 passed + 26 named skips = 405 collected; 77/84 selftests; 3/5 gates — no ceiling raised
• Zero new lint findings, measured against unmodified main with the Gate's own commands (identical ruff histograms)
• No live SQL constant still contains ORDER BY r.ts DESC LIMIT 1 (AST-verified; the three textual hits are the docstring and comments explaining the defect)
• Arm + member + profile identity preserved; no same-agent arms collapsed to a provider name
• Each feat(profiles+provenance): a profile per agent, and the provenance chain that makes per-agent identity trustworthy #42 thread answered and resolved, and feat(profiles+provenance): a profile per agent, and the provenance chain that makes per-agent identity trustworthy #42 carries a note that these fixes did not ship with its merge

_{Workflow source: review follow-up to #42. Not a new issue — #42's review is the origin, and its three inline threads carry the per-finding detail.}

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your current included review allowance is based on your included PR review attempts over the past 7 days.

Next review available in: 33 minutes

Limit details: You’ve used the included review currently available. Your 74 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 92dd8b0f-2c22-48a9-94ea-845b23321756

📥 Commits

Reviewing files that changed from the base of the PR and between fc1fd42 and 9672ffb.

📒 Files selected for processing (7)

• .verify-floor.json
• exp_abcd.py
• experiment_recovery.py
• partitioned_review.py
• research_subjects.py
• test_experiment_arm_identity.py
• test_partitioned_review.py

---

_{Comment @coderabbitai help to get the list of available commands.}

[agents-workflows-bot]

Workflow source detected

PR #62 now has valid workflow source context (origin=review_followup).

No linked GitHub issue is required for this PR.

[agents-workflows-bot]

Automated Status Summary

Head SHA: fe920fb
Latest Runs: ⏳ pending — Gate
Required: core tests (3.12): ⏳ pending, core tests (3.13): ⏳ pending, docker smoke: ⏳ pending, gate: ⏳ pending

Workflow / Job	Result	Logs
(no jobs reported)	⏳ pending	—

Updated automatically; will refresh on subsequent CI/Docker completions.

---

Keepalive checklist

Scope

No scope information available

Tasks

• No tasks defined

Acceptance criteria

• No acceptance criteria defined

[stranske]

Runner dispatch state for autofix on PR #62. Do not edit.

[stranske]

Runner dispatch state for codex on PR #62. Do not edit.

[github-actions]

Workflow state fingerprint for Agents Gate Followups. Do not edit.

[stranske]

Now fully green — and the earlier red was upstream, as measured

All 23 checks pass, Gate / gate included ("All Gate checks succeeded"). The five python ci jobs that were red earlier in this PR's life were the fleet-wide autofix-versions.env pin-file failure, fixed on main by bd6da2e5 ("the Gate was failing on a missing pin file, not on undrained lint") and picked up here by merging main. Nothing in this diff touches .github/.

That diagnosis was measured before the fix landed, not guessed. Against git archive origin/main in a scratch tree, running the Gate's exact commands on unmodified main and on this branch:

	unmodified main	this branch
black --check --line-length 100	clean, 194 files	clean, 194 files
ruff check --select E4,E7,E9,F	37 errors	37 errors
rule histogram	4×E402, 7×E731, 19×E741, 1×F601, 6×F841	identical

The only textual difference between the two error lists was two line numbers drifting in exp_abcd.py because this change adds lines above them — so this branch adds zero lint findings.

verify.py counts on the final head

tree	result
this checkout	405 passed, 0 failed, 0 skipped; 84/84 selftests; 5/5 gates
exec-mirror layout	394 passed + 11 skipped = 405 collected; 84/84 selftests; 5/5 gates — the 11 name their prerequisite (.github/workflows, docs, scripts absent: the mirror carries root-level modules only), which is exactly what main's own floor note predicts for this layout
CI	379 passed + 26 skipped = 405 collected (floor 405); 77/84 selftests; 3/5 gates — every skip naming its missing prerequisite, exactly at the agreed ceilings 26/26, 7/7, 2/2

Local and CI agree on 405 collected. The CI skips are the bare-runner profile .verify-floor.json documents (no populated capability ledger, no agent CLIs); no ceiling was raised by this branch.

The mirror run is the cmp-clean-is-not-agreement second run. The live ~/.codex/orchestrator-mirror was deliberately not synced — ORCH_MIRROR pointed at a scratch path — so CLAUDE.md §1's manual circuit breaker between an agent's change and the dispatcher stays intact.

On the floor

.verify-floor.json conflicted three times during this PR — with #59 (387→391), with the propensity work, and with the Gate fix (391→402). Each was resolved as the union of both rationales with the count re-measured on the merge result, never arithmetic on either side: 405 = 402 + this branch's 3, measured. Worth naming as a pattern rather than luck — it is the one file every concurrent branch touches, so N branches in flight produce N conflicts, and union-plus-remeasure is the only resolution that cannot leave the floor below reality.

_{Supersedes an earlier comment on this PR that reported the pre-merge counts and said "83/83 selftests"; the merge with main adds improvement_log.py, making it 84.}

[github-actions]

Workflow state fingerprint for Keepalive Loop Reporter. Do not edit.

[stranske]

Fixed: added <!-- workflow-source:review_followup --> to the PR body, which is the accurate source — this PR's origin is #42's CodeRabbit review, not a new issue. Also filled the keepalive checklist the summary reported as empty (#### Scope / #### Tasks / #### Acceptance criteria), with every box checked, since the work is complete and verified in all three trees. Deliberately no unchecked tasks: an unchecked box plus an agent:* label is what drives keepalive rounds, and there is nothing here for an agent to continue.

🤖 Addressed by Claude Code