Skip to content

docs(research): review-agent's replay seam generalises, its mechanism does not (#350) - #437

Merged
serina-mcfall merged 1 commit into
launchpadfrom
research/350-replay-generalisation
Aug 22, 2026
Merged

docs(research): review-agent's replay seam generalises, its mechanism does not (#350)#437
serina-mcfall merged 1 commit into
launchpadfrom
research/350-replay-generalisation

Conversation

@tucktuck101

Copy link
Copy Markdown
Collaborator

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

Field Value
Harness / provider Claude Code
Model claude-opus-5[1m]
Session reference N/A - the harness exposes no shareable run id
Initiating human @tucktuck101

Objective

Add launchpad/Research/350-replay-generalisation.md establishing 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 5d76799d6e44f2f76aa7bd78c5343d339af98f63 and recommendations are confined to a labelled section attributed to me rather than woven through the evidence.

Verification

Command run:

git rev-parse launchpad/launchpad
grep -nE "^def |^class |Reviewer|reviewer" launchpad/review-agent/run_dimensions.py
sed -n '133,175p' launchpad/review-agent/test_recordings.py
grep -n "recording\|RECORDING" launchpad/review-agent/test_recordings.py
sed -n '/^CONTROLS = \[/,/^\]/p' launchpad/review-agent/run_controls.py   # via #342

Raw output:

$ git rev-parse launchpad/launchpad
5d76799d6e44f2f76aa7bd78c5343d339af98f63

# run_dimensions.py
118:Reviewer = Callable[[str], object]
 19:demonstrable without them: the reviewer is an **injected callable**, defaulting to
 27:* **Reviewer signature: ``Callable[[str], dict | str]``, called as
 28:  ``reviewer(document)``.** It returns (or, if a JSON string, decodes to) a
 32:  ``head_sha``, ``completion_marker``): a reviewer's output is untrusted content,
327:def _run_dimensions_concurrently(

# test_recordings.py:133-141 -- the replay call site
                    recorded = _load_recording(fixture, dimension)
                    content = {"outcome": recorded["outcome"], "findings": recorded["findings"]}
                    doc = run_dimensions.build_document(
                        0, "a" * 40, "b" * 40, surfaces, [dimension], nonce,
                        reviewer=lambda document, content=content: content,
                    )
                    self.assertEqual(findings.validate(doc), [])

# test_recordings.py:144-162 -- how "no network" is proven
        with mock.patch("fetch.subprocess.run") as fetch_run, \
             mock.patch("run_dimensions.subprocess.run") as runner_run:
            ...
            fetch_run.assert_not_called()
            runner_run.assert_not_called()
  • Tests or checks were run and the raw output is pasted above
  • The diff is confined to the scope of the linked issue
  • No secrets, keys, tokens or hostnames were added to tracked files

Not verified

I did not execute the review agent's suites. #342 established they are absent from CI's CONTROLS list; I read test_recordings.py rather 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.py or the check_*.py controls 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_sha and the completion marker itself, visible at the call site where the test passes "a" * 40 and "b" * 40 rather 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

  1. Criterion 7 may be mis-scoped in prd: the cohort test suite — what we test that upstream doesn't #290. It lists the agent replay harness among three pieces of "genuinely unowned" scope; my reading is that the seam-injection half is already proven here and reusable, and only the keyed/ordered store is missing. Rescoping the criterion is a human call.
  2. The miss policy is a design decision with a gating consequence. What happens when replayed code requests something absent from the recording determines whether the harness can gate at all under ADR-0019, which only permits deterministic assertions to gate. I have not designed it and should not.
  3. My recommendations are marked as opinion in the document and are not attributed to any source, per the claim rule. None of the three is a decision I am entitled to take.

… does not (#350)

Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>

@serina-mcfall serina-mcfall left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:118Reviewer = 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:327def _run_dimensions_concurrently( — the "concurrent, not iterative" claim is correctly cited.
  • test_recordings.py:133-141reviewer=lambda document, content=content: content. The core claim is true: the replay callable takes document and never reads it. It is a constant function.
  • test_recordings.py:144-162 → the mock.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 .py files, 5 of them test_*, so 24 production modules including run_dimensions.py, which you did read — "other" should be 23. And the block cited as test_recordings.py:133-141 renders 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 serina-mcfall left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Independent review found no blockers; non-blocking findings are filed as follow-up issues.

@serina-mcfall
serina-mcfall merged commit 960cb63 into launchpad Aug 22, 2026
24 checks passed
@serina-mcfall
serina-mcfall deleted the research/350-replay-generalisation branch August 22, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

by:agent Filed or authored by an AI agent, not a human

Projects

None yet

Development

Successfully merging this pull request may close these issues.

task: find out whether the review-agent's record-replay pattern can express an agent that takes tool actions

2 participants