portfolio-optimiser/.claude/projects/2026-06-23-fase1-derisk-spikes/review.md

type	review_version	created	task	slug	project_dir	brief_path	scope_sha_start	scope_sha_end	reviewed_files_count	verdict	findings
trekreview	1.0	2026-06-24	Fase 1 de-risk spikes (A–D): empirically validate the framework's most dangerous assumptions before the MVP vertical slice	fase1-derisk-spikes	.claude/projects/2026-06-23-fase1-derisk-spikes/	.claude/projects/2026-06-23-fase1-derisk-spikes/brief.md	ffd3ad4	a2dff21	20	ALLOW

Review: Fase 1 de-risk spikes (A–D) — round 2 (post-remediation)

Executive Summary

Verdict: ALLOW. This is the round-2 re-review after remediation commit a2dff21. Both independent reviewers return 0 findings.

Round 1 (ffd3ad4..b81e22b) returned BLOCK on one BLOCKER: Spike B(b)'s fan-out experiment asserted a tautological per-client call_count (3 vs 1) rather than the real G2/B7 shared-Workflow state-corruption footgun, making findings-b.md's "CONFIRMED" a false-confirm. A MINOR flagged an unguarded findings-file write.

Both are now genuinely resolved, and the code-correctness reviewer verified the fix against MAF internals, not just the prose:

• BLOCKER resolved. spikes/b_footguns.py was rebuilt to observe the real mechanism — the conversation history each participant receives (new FakeChatClient.received_texts seam in spikes/_harness.py). ConcurrentBuilder.build() wraps each participant in an AgentExecutor whose _session is created once and reused across Workflow.run() calls; with no context provider, Agent.run auto-injects an InMemoryHistoryProvider and SessionContext.get_messages(include_input=True) feeds the accumulated prior-run messages to the chat client. So a reused workflow genuinely delivers run N the prompts/replies of runs 0..N-1 — measured as monotonic growth [[p0], [p0,p1], [p0,p1,p2]] — while a fresh instance per run gives each a clean thread ([[p0], [p1], [p2]]). The tests assert this by message content, non-vacuously (an empty extractor would make them fail, not pass).
• MINOR resolved. spikes/a_groupchat.py now mkdir(parents=True, exist_ok=True)s the findings dir before write_text.
• The _harness.py seam change (shared by all four spikes) introduced no new correctness defect (verified).

One transparent limitation, recorded as prose context (not a finding), carried over from round 1: SC-A's empirical deliverable is partial — the codified verdict() logic and render_comparison() exist and are tested, but the measured two-arm comparison table was not produced because run_live is skipif-gated and no LOCAL endpoint ran. The brief's Open Questions/Assumptions explicitly sanctions the skip-without-endpoint fallback and the plan ratified the endpoint-dependent split, so it is not a BROKEN_SUCCESS_CRITERION. SC-A's empirical half remains open until a LOCAL endpoint run fills findings-a.md.

Spikes A, C, D, the quality gate, and all seven Non-Goals trace cleanly to delivered code (see Coverage). No scope creep; no plan/execute drift; src/** untouched.

Coverage

File	Treatment	Reason
uv.lock	skip	lockfile (*.lock pattern)
CLAUDE.md	summary-only	reviewed (plan-sanctioned 1.8.0→1.9.0 version fix)
pyproject.toml	summary-only	reviewed
docs/fase1-spikes/README.md	summary-only	reviewed (Spike B rows updated)
docs/fase1-spikes/findings-a.md	summary-only	reviewed
docs/fase1-spikes/findings-b.md	summary-only	reviewed (§(b) + Method note rewritten)
docs/fase1-spikes/findings-c.md	summary-only	reviewed
docs/fase1-spikes/findings-d.md	summary-only	reviewed
spikes/init.py	summary-only	reviewed
spikes/_harness.py	summary-only	reviewed (received_texts + message_texts seam added)
spikes/a_groupchat.py	summary-only	reviewed (MINOR fix: mkdir before write)
spikes/b_footguns.py	summary-only	reviewed (fan-out rebuilt — BLOCKER fix)
spikes/c_validator.py	summary-only	reviewed
spikes/d_verdictstore.py	summary-only	reviewed
tests/spikes/test_a_groupchat.py	summary-only	reviewed
tests/spikes/test_b_footguns.py	summary-only	reviewed (fan-out tests rebuilt)
tests/spikes/test_c_validator.py	summary-only	reviewed
tests/spikes/test_d_verdictstore.py	summary-only	reviewed
tests/spikes/test_harness.py	summary-only	reviewed
tests/spikes/test_imports.py	summary-only	reviewed

Findings (BLOCKER)

None.

Findings (MAJOR)

None.

Findings (MINOR)

None.

Findings (SUGGESTION)

None.

Suppressed Findings

None.

Remediation Summary

• BLOCKER: 0
• MAJOR: 0
• MINOR: 0
• SUGGESTION: 0

Round-1 BLOCKER and MINOR are both genuinely resolved by commit a2dff21, verified independently against MAF internals. Fase 1 is clear to close. The one open item (SC-A empirical comparison table) is brief-sanctioned and is carried into Fase 2 as a known LOCAL-endpoint task, not a blocker.

{
  "reviewer": "review-coordinator",
  "verdict": "ALLOW",
  "counts": { "BLOCKER": 0, "MAJOR": 0, "MINOR": 0, "SUGGESTION": 0 },
  "findings": []
}