docs(research): review-agent's replay seam generalises, its mechanism does not (#350) - #437
Conversation
… does not (#350) Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>
serina-mcfall
left a comment
There was a problem hiding this comment.
No blockers — two follow-up issues filed
The central distinction survives verification, and it is a genuinely useful one. Every line citation resolves at the pinned SHA:
run_dimensions.py:118→Reviewer = Callable[[str], object]— exact.run_dimensions.py:19,:27-28,:32-34— all three docstring quotes land on the cited lines, including the trusted-field separation.run_dimensions.py:327→def _run_dimensions_concurrently(— the "concurrent, not iterative" claim is correctly cited.test_recordings.py:133-141→reviewer=lambda document, content=content: content. The core claim is true: the replay callable takesdocumentand never reads it. It is a constant function.test_recordings.py:144-162→ themock.patch(...subprocess.run)/assert_not_called()block is exactly there — and you are scrupulous in saying this proves "no subprocess ran", not that it is an HTTP-level cassette. That distinction is the note.- "15 recordings" — 5 fixtures × 3 dimensions. Exact.
I reviewed this one knowing the review-agent code is largely the requesting maintainer's own, and judged the claims on evidence rather than deference. They hold.
Filed, not blocking
-
#441 — Recommendations §3 says "ADR-0019 only permits a deterministic assertion to gate". ADR-0019 does not exist at the SHA this note opens by pinning everything to; it is unmerged PR #281. The paraphrase is faithful to that PR, so this is a forward reference rather than a fabrication — and it already sits inside a block you fenced as "Opinion, mine… Not established by any source above." That fencing is why it is Medium and not higher, and it is the pattern the two other notes with this problem should copy. One clause — "per the pending ADR-0019 (#281)" — closes it.
-
#449 — two Lows, grouped with nits from other notes. "The other 24 production modules in the directory were not read": there are 29
.pyfiles, 5 of themtest_*, so 24 production modules includingrun_dimensions.py, which you did read — "other" should be 23. And the block cited astest_recordings.py:133-141renders only through 139, with 140-141 dropped and no ellipsis.
One observation that strengthens your thesis, currently unstated
The note says "nothing binds a recorded output to the input that produced it." Strictly, the recordings are keyed by (fixture, dimension) via _load_recording(fixture, dimension), and the recording's _provenance.seed is used to build the nonce that goes into the document.
So a binding exists — but it runs recording → input, which is the reverse of what replay needs to be meaningful. That reversal is a sharper statement of your point than the current wording, and it is worth a sentence: the seam does not merely lack a binding, it has one pointing the wrong way.
Reviewed at head 20c91350b. This is a review, not an approval — approval is not mine to give.
🤖 Review drafted by Claude Code (claude-opus-5) for @serina-mcfall.
serina-mcfall
left a comment
There was a problem hiding this comment.
Approved. Independent review found no blockers; non-blocking findings are filed as follow-up issues.
Summary
Answers #350 by reading the review agent's replay path. Its seam is right — the model is an injected callable and real runner code executes with no live inference — but the replay is
lambda document, content=content: content, a constant function that discards its input. There is no request keying, no ordering and no representable cache miss, so the architecture generalises to an acting agent while the mechanism does not.Related issue
Closes #350
Issue type
Task
Agent provenance
Objective
Add
launchpad/Research/350-replay-generalisation.mdestablishing whether the existing record-replay pattern can express an agent that takes tool actions, and what it would need if not.Impacted components
launchpad/Research/350-replay-generalisation.md
Approach and rejected alternatives
Located the substitution point by type signature rather than by reading the whole directory:
Reviewer = Callable[[str], object]fixes the entire model surface at one line, and the replay call site then answers the question directly. That is why the document cites four specific line ranges instead of summarising 24 modules.Rejected answering from the docstrings alone.
test_recordings.py's header says recordings are "replayed through the real runner" and that "replay makes no network call", both of which are true and both of which would support a much more optimistic answer than the call site justifies. The lambda is the fact that settles it.This is the first document written under the new citation rule, so every reference is pinned to
5d76799d6e44f2f76aa7bd78c5343d339af98f63and recommendations are confined to a labelled section attributed to me rather than woven through the evidence.Verification
Command run:
Raw output:
Not verified
I did not execute the review agent's suites. #342 established they are absent from CI's
CONTROLSlist; I readtest_recordings.pyrather than running it, so this reports what it asserts, not that it currently passes.The other 24 production modules in the directory were not read. Something in
contain.py,detect.pyor thecheck_*.pycontrols could contain request-keyed machinery I did not find — my search was directed at the reviewer seam and the recordings.No external prior art was surveyed. The gap analysis reasons from this codebase alone; comparison against established record-replay harnesses is closer to #348 and is not attempted here.
Whether the 15 recordings contain anything that should not be public is #348's question and is untouched by this document.
Security implications
None added. The document surfaces one existing property worth a reviewer's attention: the runner treats reviewer output as untrusted and assembles
head_sha,base_shaand the completion marker itself, visible at the call site where the test passes"a" * 40and"b" * 40rather than reading them from the recording. A recorded agent transcript would be untrusted input in exactly the same way, so that separation is the property most worth preserving in anything built next.Escalations