diff --git a/docs/fase1-spikes/README.md b/docs/fase1-spikes/README.md index 8fbe505..360814e 100644 --- a/docs/fase1-spikes/README.md +++ b/docs/fase1-spikes/README.md @@ -13,7 +13,7 @@ the four assumptions that — if wrong — force a redesign: | Spike | Assumption (register ref) | What it measures | |-------|---------------------------|------------------| | **A** | Group Chat maker-checker beats a single-agent baseline by enough to justify its multiplicative token cost (U3 / G7) | convergence rounds, stall frequency, token use — maker-checker vs single-agent, with a cheaper/better verdict | -| **B** | The known MAF footguns behave as predicted and our guards hold: Magentic unbounded termination when `limits=None` (G1/B4); shared-builder / fan-out state corruption (G2/B7) | guard fires on unbounded Magentic; zero state-bleed with the fresh-instance helper | +| **B** | The known MAF footguns behave as predicted and our guards hold: Magentic unbounded termination when `limits=None` (G1/B4); shared-builder / fan-out state corruption (G2/B7) | guard fires on unbounded Magentic; a reused `Workflow` accumulates conversation across runs (project N contaminates N+1) while the fresh-instance helper gives each run a clean thread — measured by received-message content, not a call counter | | **C** | A blocking deterministic hybrid-validator (B1) can *structurally* block an out-of-range proposal | structural rejection of an out-of-range proposal; P10/P50/P90 for a valid one; capped self-repair | | **D** | ExpeL retrieval (B2) surfaces a relevant prior verdict for a similar new proposal | top-K retrieval returns the structurally-similar verdict over surface-text decoys | @@ -50,7 +50,7 @@ The gate stays green from the logic layer alone. | Spike | Assumption | Result | Verdict | Token use | Implication for Fase 2 | |-------|-----------|--------|---------|-----------|------------------------| | **A** | maker-checker > single-agent (U3/G7) | verdict logic green; `GroupChatBuilder` drivable; cheaper/better is endpoint-dependent | **CONFIRMED (logic)** — empirical arm gated | word-count proxy; live arm not run (no endpoint) | Keep the codified `verdict` (better ∧ ≤3× tokens); measure the empirical cost/benefit on a LOCAL endpoint before locking the debate default | -| **B** | Magentic unbounded + fan-out bleed (G1/G2) | unbounded `max_round_count=None` needs an external guard; shared fan-out instance bleeds, fresh does not | **CONFIRMED** | 0 — no live LLM | Require explicit round/stop caps for any Magentic loop (fail-fast); use a fresh-instance-per-run factory for fan-out (B7) | +| **B** | Magentic unbounded + fan-out bleed (G1/G2) | unbounded `max_round_count=None` needs an external guard; a reused `Workflow` accumulates the conversation thread across runs (project N's prompts/replies leak into N+1), a fresh instance per run does not — measured by received-message content | **CONFIRMED** | 0 — no live LLM | Require explicit round/stop caps for any Magentic loop (fail-fast); use a fresh-instance-per-run factory for fan-out (B7) | | **C** | blocking hybrid-validator (B1) | typed IR + real CBC solve + Monte-Carlo P10/P50/P90; out-of-range → `Rejection` (distinct type, no percentiles) | **CONFIRMED** | 0 — deterministic; live gen gated | Keep the `Rejection`/`ValidatedProposal` type split (structural block) + CBC-absent escalate; migrate to `pulp[cbc]`/`COIN_CMD` for PuLP 4.0 | | **D** | ExpeL retrieval (B2) | structural similarity (codes + measure + magnitude) returns the true match as top-1 over surface-text decoys; deterministic | **CONFIRMED** | 0 — deterministic retrieval | Keep structured similarity as the baseline; add embeddings only if it proves insufficient on real data | diff --git a/docs/fase1-spikes/findings-b.md b/docs/fase1-spikes/findings-b.md index 2e5d1c6..12c7d80 100644 --- a/docs/fase1-spikes/findings-b.md +++ b/docs/fase1-spikes/findings-b.md @@ -29,16 +29,41 @@ in-flight fake response surfaces a `RuntimeWarning: coroutine was never awaited` cosmetic (no functional effect); the spike silences the OTel context logger and filters the warning. Worth knowing if Fase 2 observes streamed orchestrations. -## (b) Fan-out state isolation — CONFIRMED +## (b) Fan-out state isolation — CONFIRMED (conversation-history bleed) -- **Shared instance:** one `ConcurrentBuilder` workflow reused across the three reference - projects → the shared fake clients accumulate calls (max call count == 3) → **state - bleed**. -- **Fresh instance:** `fresh_workflow()` builds a new workflow + fresh clients per run → - each run's clients see exactly one call (`[1, 1, 1]`) → **zero bleed** across all 3 - projects (B7). +The observable is the **message history each participant receives** (captured via +`FakeChatClient.received_texts`), NOT a call counter. A counter rises for any reused +mutable object and proves nothing about MAF; message content proves the workflow carried +state across runs. (An earlier version of this spike measured only a call counter and was +a tautology — caught and rebuilt; see the Method note below.) + +- **Shared instance:** one built `ConcurrentBuilder` workflow reused across the three + reference projects. MAF **accumulates the shared conversation thread across `.run()` + calls**: each run's participants receive the PRIOR projects' prompts and replies too. + Measured (project ids visible per run, in order): + `[[FV42-GSV-E1], [FV42-GSV-E1, RV13-RAS-TP], [FV42-GSV-E1, RV13-RAS-TP, BRU-LAKS-REHAB]]` + — strictly monotonic growth → run N is contaminated by runs 0..N-1 → **genuine state + bleed (G2/B7)**. +- **Fresh instance:** `fresh_workflow()` builds a new workflow + clean thread per run → + each participant sees ONLY its own project: `[[FV42-GSV-E1], [RV13-RAS-TP], + [BRU-LAKS-REHAB]]` → **zero cross-run contamination** (B7 mitigation works). - **Implication:** Fase 2 fan-out must build a fresh workflow/executor instance per - project run (a factory like `fresh_workflow()`), never reuse a shared instance. + project run (a factory like `fresh_workflow()`), never reuse a shared instance — a + reused `Workflow` leaks one project's context into the next, which would corrupt + per-project cost analyses. This is a property of MAF's `Workflow` thread state, not of + the participants. + +### Method note (why this was rebuilt) + +The first cut of this experiment asserted that a reused instance's per-client `call_count` +reached 3 while a fresh instance's stayed at 1. That is a tautology: any un-reset mutable +counter rises across reuse, independent of MAF, so it never demonstrated the footgun. +`/trekreview` flagged it as a `BROKEN_SUCCESS_CRITERION` (false-confirm of a de-risk +assumption). The rebuilt experiment observes real MAF thread state via the messages each +participant receives, so a passing test now genuinely means "the reused workflow carried +project N's conversation into project N+1." Lesson carried into Fase 2: **a de-risk +assertion must observe the mechanism it claims to test, not a proxy that moves for unrelated +reasons.** ## Token use diff --git a/spikes/_harness.py b/spikes/_harness.py index 3a63295..0cc7d1d 100644 --- a/spikes/_harness.py +++ b/spikes/_harness.py @@ -99,6 +99,26 @@ def _word_tokens(text: str) -> int: return len(text.split()) +def message_texts(messages: Sequence[Message]) -> list[str]: + """Extract the text payloads from a sequence of MAF ``Message`` objects. + + MAF content items vary (objects with ``.text``, bare strings, or ``{"text": ...}`` + dicts); this normalizes them to a flat list of strings. Used by ``FakeChatClient`` + to record exactly what an agent *received* per call — the observable Spike B uses to + detect cross-run conversation bleed (G2/B7).""" + out: list[str] = [] + for m in messages: + for c in getattr(m, "contents", []) or []: + text = getattr(c, "text", None) + if text is None and isinstance(c, str): + text = c + if text is None and isinstance(c, dict): + text = c.get("text") + if text is not None: + out.append(str(text)) + return out + + class FakeChatClient(BaseChatClient): """A deterministic, network-free ``BaseChatClient`` for driving MAF agents in tests. @@ -116,6 +136,9 @@ class FakeChatClient(BaseChatClient): self._default = default_reply self.total_tokens = 0 self.call_count = 0 + # One entry per call: the text payloads this client RECEIVED that call. Lets a + # spike observe whether a reused workflow feeds run N+1 the prior runs' history. + self.received_texts: list[list[str]] = [] def _next_reply(self) -> str: reply = self._scripted[self._idx] if self._idx < len(self._scripted) else self._default @@ -134,6 +157,7 @@ class FakeChatClient(BaseChatClient): ) -> Any: # Matches the GA BaseChatClient contract: return a ResponseStream when # streaming, otherwise an awaitable resolving to a ChatResponse. + self.received_texts.append(message_texts(messages)) reply = self._next_reply() if stream: diff --git a/spikes/a_groupchat.py b/spikes/a_groupchat.py index cee0bb2..a4cf366 100644 --- a/spikes/a_groupchat.py +++ b/spikes/a_groupchat.py @@ -176,6 +176,10 @@ async def run_live( table = render_comparison(mc, single) if write_findings: + # Guard the parent dir: the spike may run after the documented disposal + # (`rm -rf docs/fase1-spikes`), and a missing dir must not discard the + # just-measured live result with a bare FileNotFoundError. + _FINDINGS.parent.mkdir(parents=True, exist_ok=True) _FINDINGS.write_text(_findings_doc(table), encoding="utf-8") return {"mc": mc, "single": single, "verdict": verdict(mc, single), "table": table} diff --git a/spikes/b_footguns.py b/spikes/b_footguns.py index 1ada2a8..86fb499 100644 --- a/spikes/b_footguns.py +++ b/spikes/b_footguns.py @@ -10,9 +10,14 @@ the experiment here — no live LLM, so this whole spike runs in the quality gat round/iteration guard and confirm the guard is what stops it (it does NOT self-terminate). With an explicit `max_round_count` it terminates cleanly. -(b) **Fan-out state isolation (G2/B7).** Reusing ONE workflow instance across the three - reference projects bleeds state (the shared fake clients accumulate calls); a FRESH - instance per run — via `fresh_workflow()` — shows zero bleed. +(b) **Fan-out state isolation (G2/B7).** Reusing ONE built `ConcurrentBuilder` workflow + across the three reference projects bleeds *conversation state*: MAF accumulates the + shared thread across `.run()` calls, so each run's participants receive the PRIOR + projects' prompts and replies (project N contaminates N+1's context). A FRESH instance + per run — via `fresh_workflow()` — gives each run a clean thread (zero contamination). + The observable is the message history each participant *receives*, captured via + `FakeChatClient.received_texts` — NOT a call counter (a counter would rise for any + reused mutable object and prove nothing about MAF state). Token use: 0 — no live LLM (the fake client's "tokens" are word-counts of canned replies). """ @@ -101,22 +106,41 @@ def fresh_workflow() -> tuple[object, tuple[FakeChatClient, FakeChatClient]]: return workflow, (c1, c2) -async def shared_instance_max_calls(project_ids: list[str]) -> int: - """Reuse ONE fan-out instance across every project — state bleeds: the shared clients - accumulate calls across runs. Returns the max per-client call count (== len once - bled).""" +def _projects_seen(received_texts: list[str], project_ids: list[str]) -> list[str]: + """Which project ids appear anywhere in the messages an agent received on one call. + + The observable for genuine state bleed: if a later run's agent sees an EARLIER + project's id, the workflow carried that project's conversation forward.""" + blob = " ".join(received_texts) + return [pid for pid in project_ids if pid in blob] + + +async def shared_instance_conversation_bleed(project_ids: list[str]) -> list[list[str]]: + """Reuse ONE built fan-out workflow across every project. MAF accumulates the shared + thread across `.run()` calls, so run N's participants also receive runs 0..N-1's + prompts/replies — genuine cross-run state corruption (G2/B7). + + Returns, per run (in order), which project ids were visible to a participant on that + run. With a reused instance this grows monotonically: ``[[p0], [p0, p1], [p0, p1, p2]]``.""" workflow, clients = fresh_workflow() for pid in project_ids: await workflow.run(f"Evaluate project {pid}.") - return max(c.call_count for c in clients) + # One participant is representative: concurrent fan-out feeds every participant the + # same accumulated thread. clients[0] was called once per run, in order. + rep = clients[0] + return [_projects_seen(call_view, project_ids) for call_view in rep.received_texts] -async def fresh_instance_call_counts(project_ids: list[str]) -> list[int]: - """A FRESH instance per project — zero bleed: every run's clients see exactly one - call. Returns the per-run max call count (each should be 1).""" - counts: list[int] = [] +async def fresh_instance_conversation_isolation(project_ids: list[str]) -> list[list[str]]: + """A FRESH instance per project (the B7 mitigation): each run gets a clean thread, so + a participant sees ONLY its own project — zero cross-run contamination. + + Returns, per run, the project ids visible to a participant; each should be exactly + its own: ``[[p0], [p1], [p2]]``.""" + seen_per_run: list[list[str]] = [] for pid in project_ids: workflow, clients = fresh_workflow() await workflow.run(f"Evaluate project {pid}.") - counts.append(max(c.call_count for c in clients)) - return counts + # fresh client -> exactly one call this run; read its single received view. + seen_per_run.append(_projects_seen(clients[0].received_texts[0], project_ids)) + return seen_per_run diff --git a/tests/spikes/test_b_footguns.py b/tests/spikes/test_b_footguns.py index 5806a39..a59a8b9 100644 --- a/tests/spikes/test_b_footguns.py +++ b/tests/spikes/test_b_footguns.py @@ -7,8 +7,8 @@ Step 2 builder smoke. Pattern: tests/test_backends.py. from portfolio_optimiser.reference_domain import load_reference_projects from spikes.b_footguns import ( bounded_magentic_terminates, - fresh_instance_call_counts, - shared_instance_max_calls, + fresh_instance_conversation_isolation, + shared_instance_conversation_bleed, unbounded_magentic_self_terminates, ) @@ -24,16 +24,22 @@ async def test_bounded_magentic_terminates_within_limit() -> None: assert await bounded_magentic_terminates(max_round_count=2) is True -async def test_shared_instance_bleeds_state_across_projects() -> None: +async def test_shared_instance_bleeds_conversation_across_projects() -> None: ids = [p.id for p in load_reference_projects()] assert len(ids) == 3 - bled = await shared_instance_max_calls(ids) - # One shared instance reused across all 3 projects -> calls accumulate -> bleed. - assert bled == len(ids) + seen = await shared_instance_conversation_bleed(ids) + # Reusing ONE built workflow accumulates the MAF thread across runs: each run's + # participant receives the EARLIER projects' prompts too (genuine G2/B7 bleed), + # measured by message CONTENT — not a call counter. + assert seen[0] == [ids[0]] # first run is clean — nothing prior to bleed + assert set(seen[-1]) == set(ids) # last run saw every project's prompt + # Strictly monotonic growth = conversation history accumulating, not coincidence. + assert all(set(seen[i]) < set(seen[i + 1]) for i in range(len(seen) - 1)) -async def test_fresh_instance_zero_bleed_across_projects() -> None: +async def test_fresh_instance_isolates_conversation_across_projects() -> None: ids = [p.id for p in load_reference_projects()] - counts = await fresh_instance_call_counts(ids) - # A fresh instance per project -> each run sees exactly one call -> zero bleed. - assert counts == [1, 1, 1] + seen = await fresh_instance_conversation_isolation(ids) + # A fresh instance per project gives each run a clean thread -> a participant sees + # ONLY its own project -> zero cross-run contamination (the B7 mitigation works). + assert seen == [[pid] for pid in ids]