# Round 3 adversarial review — fix/issue-15873 @ 7dcb809db870aa1903a6031cdf93b6155ac8124d

Diff reviewed: b5a226e49c..7dcb809db8 (9 files: 3 production, 3 tests, 3 fixture files). The ed9b7ca9c5 Node pin is unrelated and was excluded.

## Confirmed findings

1. **Blocks merge (repository policy, not code).** Commit [7dcb809](scripts/fixtures/issue-15873) adds `scripts/fixtures/issue-15873/{seed.py,runtime.json,README.md}`. That is about 990 lines of Python and JSON used only to replay evidence for this issue.
   - Nothing in the repository references it, and no test or CI step runs it.
   - `scripts/` has no `fixtures/` directory and no Python anywhere else. Repository tooling is TypeScript.
   - The README itself says it supports "the separate evidence index".
   - AGENTS.md says "Never commit PR-only screenshots or assets" and "Do not commit … research notes, or agent scratch files". This is PR-only evidence tooling.
   - **Smallest fix:** drop 7dcb809 from the branch. Publish the three files next to the evidence index (for example with ghdrop) and link them from the PR body's reproduction step. The index currently says "run the committed `seed.py`", so update that wording.
   - If Julius wants a durable replay fixture, it should be a TypeScript test seed run by the suite, which is out of scope here.

No product code bugs found.

## Minor notes (non-blocking)

- seed.py, if it is kept anywhere:
  - `ids = {...}` is unused.
  - `p['nodeId'] = data[...]['payload_json'] and json.loads(...)` uses `and` as a ternary.
  - The import order is unsorted.
  - These do not matter once the file is out of the repository.
- The safety guard in seed.py is correct: it rejects anything under `~/.t3`, requires `.t3` in the path and `statev2.sqlite` as the file name, and needs Python 3.9 or later for `is_relative_to`.
- `runtime.json` contains no credentials, personal paths, tailnet hosts or emails. `cwd` and `workspace_root` are `/tmp/t3-15873-workspace`. Token hits are only `tokenUsage` field names.

## Checklist

1. **Worth having and shape.**
   - Yes, it fixes a real data-loss-looking bug: a persisted late answer was missing from the timeline.
   - It is the smallest shape: a folded answer is matched by (messageId, runId) instead of messageId alone. The same edit is made in 3 places (web `session-logic.ts`, mobile `threadActivity.ts`, server `ThreadFind.ts`, both the scan and load paths).
   - No schema, contract or provider adapter changes.
   - Product decision as instructed: an answer is folded only when it lands in the same run. Any other run, including steering into a different active run, shows a bubble, and the question card keeps its history.
   - Consequence: in the different-run case the answer text appears twice, once in the collapsed card and once in the bubble. That is the intended design per the user's instruction, not a bug.

2. **Concrete bugs.** None. Traced:
   - `runId` is null on both items: still folds (`null === null`), the same as before.
   - Server load path: `json_extract('$.runId')` on JSON null or a missing key gives SQL NULL, so `null`, which matches the decoded `item.runId`.
   - Find excludes `user_input_request` rows, so the different-run case counts 1 match (the bubble), not 2.
   - `retainedMessageIds` still includes every folded key, so an anchored optimistic `async-answer:` message never duplicates the committed one. This is unchanged from before.
   - Mobile prefix cache: `mapsEqual` compares runId values, so a run change on an answer forces a full rebuild.

3. **Sensitive semantics.** No changes to auth, credentials, secrets, permissions, approvals, modes, sandbox, trust or MCP. NEEDS_JULIUS does not apply.

4. **Regex, parser and loop complexity.**
   - Production code adds no regex or parser. It is O(N) Map building and lookups, replacing the equivalent Set.
   - The new fixture regex in `seed.py` runs offline only. Benchmarks on 3000-unit malformed inputs (`python3 -I`):

     | Input | Size | Time |
     |---|---|---|
     | 12k hex characters | 12k | 0.49 ms |
     | 3000 near-UUIDs with one bad character | 111k | 6.6 ms |
     | 3000 `msg_` tokens | 75k | 1.3 ms |
     | 12k dashes | 12k | 0.58 ms |

     Both alternations are linear with no nested quantifiers. Token substitution is O(T·N) on fixed input. It never runs at product runtime.
   - `fixtures()` builds all 5 fixtures in 0.1 s and passes its own `validate()`. Turn-item IDs are unique across fixtures.
   - The intent checks hold: G, A, L and S place the answer in a different run from the question (S uses `inputIntent` steer), and C places it in the same run.

5. **Evidence vs diff.**
   - Matches the diff. I viewed before/after G-390-light: before, the "LATE ANSWER PROBE: Blue" bubble is missing between RUN N+1 FINISHED and RUN N+2 ACKNOWLEDGED. After, it is present and the question card still keeps the answer.
   - The index records Find going from 0/0 to 1/1, which is consistent with the code.
   - I did not re-run the pixel comparison or check the recordings; proof audit pass 2 owns that.
   - The only evidence-to-diff mismatch is that the index tells reviewers to run the *committed* seed.py (see finding 1).

6. **Tests.**
   - The touched files pass at 7dcb809: web 49, mobile 37, server 19, so 105/105 via `node_modules/.bin/vp test run`.
   - Existing tests cover same-run folding, different-run visibility and Find on both server paths.
   - No missed behavior that would justify a new test.

7. **Compatibility.**
   - Existing persisted data needs no migration, since `runId` was already in every turn-item payload.
   - Older clients against a new server: the server Find change only adds matches. An old client still hides the bubble, which is the pre-existing bug and not a regression.
   - A new client against an old server renders the bubble from the items it already receives.
   - Reconnect and cancel are unaffected, and the change is platform-neutral.

## Not covered

- Native iOS and Android, Electron, hot store mutation, live steering into an active run, history pagination of Find where the request and the answer fall on different pages (pre-existing behavior), and recordings. The evidence index already lists these as gaps.

## Summary

The code is ready. The only blocker is that 7dcb809 commits PR-only evidence tooling, which repository policy forbids. Drop it, host the fixture with the evidence, and the branch is ready to merge on code. The remaining surface gaps are evidence limits, not code defects.

FRICTION
tooling|`vp` was not on PATH in the review shell (no `~/.local/share/vite-plus/bin`), so the first test attempt silently printed nothing|call `<worktree>/node_modules/.bin/vp` directly in review briefs
process|evidence fixtures were committed to the product branch despite the AGENTS.md rule against PR-only assets|decide where evidence fixtures go (uploaded or committed) before capture starts, not at final review

VERDICT: FIX_FIRST