docs(s33): document the wave snapshot's deliberate intra-wave semantics, pinned by test
Closes OQ1 (SC8). At k>1 every project in a wave reads the WAVE-START store, so a verdict captured by project A does not reach project B's hypothesis prompt inside the same wave — sequentially it would. Deliberate: the snapshot is what removes the append race, and restoring intra-wave visibility would restore the completion-order dependence the barrier exists to eliminate. Learning flows ACROSS wave boundaries, not within them; concurrency trades learning granularity for wall-clock. The shipped fixture cannot show this (the fold is bundle_dir-gated and no reference project sets bundle_dir), which is a property of the fixture and not of the design — so it is pinned on the road-k + bundle-k+1 pair via the existing load_reference_projects monkeypatch seam, no new production seam. k=1 (two waves) carries the sentinel; k=2 (one wave) does not; store content stays identical across k. Each half is the other's control. Detach measured: remove _wave_snapshot -> RED on three tests including this one. Also corrects two stale docstrings that outlived Session 1's finding: the module header and the Step-2 test still named a sorted() in _merge_wave as the detach point. There is no sorted() there, and a project_id sort would BREAK the k=1-identity contract rather than protect it. A docstring naming a detach point that does not exist is the green-but-dead defect this file exists to prevent, so both now name the snapshot and mark the plan's claim as measured wrong. _wave_snapshot gains the honesty line about copying exactly two fields, with the reason no field-count guard was added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015vYbqW4MppACRvPhEMDpvF
This commit is contained in:
parent
796f8d3af0
commit
16e1734264
2 changed files with 144 additions and 10 deletions
|
|
@ -592,7 +592,14 @@ def _wave_snapshot(store: VerdictStore) -> VerdictStore:
|
|||
|
||||
``retriever`` is carried across deliberately: it is the S3.1 opt-in seam, and a snapshot that
|
||||
dropped it would silently downgrade a caller-owned store's semantic retrieval to the
|
||||
structural default mid-pass."""
|
||||
structural default mid-pass.
|
||||
|
||||
**Honesty boundary: this copies exactly two fields because ``VerdictStore`` HAS exactly two.**
|
||||
A third field added later would be silently dropped here — the same defect class as the
|
||||
``retriever`` omission this function was first written with, which the Step-2 contract test
|
||||
caught. It is left as a documented hazard rather than a guard: an assertion on the field count
|
||||
would go red on every benign addition to ``VerdictStore``, which trains people to edit the
|
||||
guard rather than think about the snapshot — a worse outcome than the line you are reading."""
|
||||
return VerdictStore(verdicts=list(store.verdicts), retriever=store.retriever)
|
||||
|
||||
|
||||
|
|
@ -702,7 +709,25 @@ async def run_portfolio(
|
|||
the reason is that the ledger is STATIC during a pass (C3: no realization happens on the run
|
||||
path, which ``test_concurrent_pass_does_not_write_on_the_run_path`` pins), so every check reads
|
||||
the same accumulated sum regardless of when it runs. Wave-assembly placement buys the
|
||||
never-started property, not determinism; determinism is the one-writer rule's."""
|
||||
never-started property, not determinism; determinism is the one-writer rule's.
|
||||
|
||||
**The one semantic difference ``k > 1`` introduces, stated plainly (OQ1).** Every project in a
|
||||
wave reads the WAVE-START store, so a verdict captured by project A does NOT reach project B's
|
||||
hypothesis prompt when A and B share a wave — sequentially it would. This is a deliberate trade,
|
||||
not an oversight: the snapshot is what removes the append race, and restoring intra-wave
|
||||
visibility would restore exactly the completion-order dependence the barrier exists to
|
||||
eliminate (what B saw would depend on whether A happened to finish first). Learning therefore
|
||||
flows ACROSS wave boundaries, not within them, and ``concurrency`` is the knob that trades
|
||||
learning granularity for wall-clock — at ``k=1`` nothing changes, and at ``k=len(portfolio)``
|
||||
the pass learns nothing from itself.
|
||||
|
||||
On the SHIPPED reference fixture this difference is invisible in outcomes, because the Step-1
|
||||
ExpeL fold is ``bundle_dir``-gated and no reference project sets ``bundle_dir`` — the chain is
|
||||
live for store CONTENT but inert for OUTCOMES. That is a property of the fixture, never a
|
||||
property of the design, so it is pinned on a bundle-backed pair where the fold does fire
|
||||
(``test_intra_wave_visibility_is_the_documented_semantic_difference``): same fixture, same
|
||||
sentinel, only the wave boundary moves. Store content stays identical across ``k`` — the
|
||||
difference is confined to what each project READ, never to what the pass produced or persisted."""
|
||||
if concurrency < 1:
|
||||
raise ValueError(
|
||||
f"concurrency must be >= 1, got {concurrency}: a non-positive wave size would run no "
|
||||
|
|
|
|||
|
|
@ -6,11 +6,21 @@ concurrently WITHOUT spending the property the whole suite rests on: determinism
|
|||
The design rests on one measured fact (``verdicts.py:303-304``): the only order-sensitive shared
|
||||
state in the pass is ``VerdictStore.verdicts`` — ONE list with ONE append site. Retrieval ranking
|
||||
is already order-independent (``verdicts.py:274-279`` ranks on ``(-similarity, id)``). So a
|
||||
per-wave snapshot plus a merge barrier that sorts each wave's new verdicts on ``project_id``
|
||||
restores byte-identity exactly — nothing wider is needed, and nothing narrower suffices.
|
||||
per-wave snapshot plus a merge barrier that preserves wave-SUBMISSION order restores byte-identity
|
||||
exactly — nothing wider is needed, and nothing narrower suffices.
|
||||
|
||||
This file grows across the plan's six steps. Step 1 covers the partitioning helper and the
|
||||
fail-fast; the determinism contract and its probe arrive in Step 2.
|
||||
**The plan said "sort each wave's new verdicts on ``project_id``"; that was measured wrong and the
|
||||
correction is load-bearing, so it is recorded here rather than only in git.** Lexicographic order
|
||||
is BRU/FV42/RV13 while the sequential pass yields FV42/RV13/BRU — a ``project_id`` sort is
|
||||
deterministic yet NOT identical to ``concurrency=1``, and the second is the actual contract. The
|
||||
plan also named that sort as the detach point; it is not. With per-project snapshots the wave list
|
||||
is never reordered by completion, so a ``sorted(...)`` in the barrier would re-sort an already
|
||||
ordered list and read as a guard while guarding nothing. **The SNAPSHOT is the half that carries
|
||||
the load** — measured RED in Session 1, and again in Step 6 against the intra-wave visibility test.
|
||||
|
||||
Each test below names the detach point it was actually MEASURED against, and — where a plausible
|
||||
detach turned out to be green — says so explicitly. A detach point that is only asserted is the
|
||||
green-but-dead failure this file exists to prevent.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -26,6 +36,11 @@ import pytest
|
|||
from conftest import _PORTFOLIO_DEFAULT_REPLY, _ProjectAwareUsageChatClient
|
||||
from test_ledger import _prefilled
|
||||
from test_portfolio import REPLIES
|
||||
from test_portfolio_learning_loadbearing import (
|
||||
_ALIGNED_REPLY,
|
||||
_bundle_kplus1,
|
||||
_road_k,
|
||||
)
|
||||
|
||||
from portfolio_optimiser import ledger as ledger_mod
|
||||
from portfolio_optimiser import verdicts as verdicts_mod
|
||||
|
|
@ -212,9 +227,16 @@ async def test_concurrent_pass_is_byte_identical_to_sequential() -> None:
|
|||
"""S3.3's core contract: ``concurrency=3`` produces the SAME store-verdict id SEQUENCE as
|
||||
``concurrency=1``, under a completion order that genuinely differs from submission order.
|
||||
|
||||
**Detach point: remove the ``sorted(...)`` from ``_merge_wave`` -> this test goes RED.**
|
||||
That is the seam. Without the sort, each wave's verdicts land in completion order, which the
|
||||
probe has made differ from submission order, and the two id sequences diverge.
|
||||
**Detach point: remove ``_wave_snapshot`` (hand every project the shared store) -> this test
|
||||
goes RED (measured, twice — Session 1 and again in Step 6).** Without the snapshot both
|
||||
projects append to one list and the verdicts land in completion order, which the probe has made
|
||||
differ from submission order, so the two id sequences diverge. Reordering the wave list on its
|
||||
way to ``_merge_wave`` is the second measured detach.
|
||||
|
||||
**NOT the detach point, despite what the plan said:** a ``sorted(...)`` inside ``_merge_wave``.
|
||||
See this module's docstring — with per-project snapshots the barrier receives an
|
||||
already-ordered list, so a sort there guards nothing, and sorting on ``project_id``
|
||||
specifically would BREAK the contract rather than protect it.
|
||||
|
||||
**Probe mechanism.** ``_OrderProbeClient`` returns a coroutine that yields to the loop N times
|
||||
(``asyncio.sleep(0)``, a pure scheduler yield — no wall clock) before delegating to the
|
||||
|
|
@ -685,3 +707,90 @@ async def test_concurrent_pass_does_not_write_on_the_run_path() -> None:
|
|||
assert len(result.runs) == 3, (
|
||||
"the pass must still have completed — an empty pass proves nothing"
|
||||
)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------------------------
|
||||
# Step 6 — the one semantic difference the wave model DOES introduce, made observable and pinned.
|
||||
# --------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
async def test_intra_wave_visibility_is_the_documented_semantic_difference(
|
||||
tmp_path, monkeypatch, make_recording_client_factory
|
||||
) -> None:
|
||||
"""OQ1, closed by measurement (SC8): at ``k > 1`` a verdict captured by project A does NOT reach
|
||||
project B's hypothesis prompt when both sit in the SAME wave — sequentially it would.
|
||||
|
||||
This is the ONE behavioural difference the wave model introduces, and it follows directly from
|
||||
the snapshot: every project in a wave reads the wave-start store, so a sibling's verdict is by
|
||||
construction not visible to it. That is a deliberate trade — the snapshot is what removes the
|
||||
append race — and it must never become silent drift, hence this test.
|
||||
|
||||
**Why the shipped fixture cannot show it, and why this test needs its own.** The Step-1 ExpeL
|
||||
fold is ``bundle_dir``-gated, and no project in ``reference_projects.json`` sets ``bundle_dir``,
|
||||
so on that fixture the cross-project chain is live for store CONTENT but inert for OUTCOMES —
|
||||
the difference exists and is unobservable. The road-*k* + bundle-*(k+1)* pair from
|
||||
``test_portfolio_learning_loadbearing.py`` is the fixture where the fold actually fires, injected
|
||||
through the SAME ``load_reference_projects`` monkeypatch seam that file uses (``:163``). No
|
||||
``projects=`` parameter is added to production code for this: that would be a new public seam
|
||||
invented for a test, past NG8.
|
||||
|
||||
**The two halves are each other's control, which is what makes this load-bearing rather than an
|
||||
observation.** ``k=1`` puts the pair in two waves and the sentinel DOES arrive; ``k=2`` puts
|
||||
them in one wave and it does NOT. Same fixture, same sentinel, same client — only the wave
|
||||
boundary moves. The ``k=1`` half fails if the fixture or the fold ever breaks, so an absence in
|
||||
the ``k=2`` half can be attributed to the wave boundary rather than to a dead test.
|
||||
|
||||
**Detach point: remove the snapshot (hand every project the shared store) -> the k=2 half goes
|
||||
RED**, because the sentinel starts reaching project B whenever B happens to run after A —
|
||||
which also reintroduces exactly the completion-order dependence the barrier exists to remove."""
|
||||
sentinel = "SENTINEL-INTRAWAVE-8d21f0 realiseringskorreksjon fra prosjekt k"
|
||||
|
||||
# Built ONCE and closed over, exactly as the reference test does. This is not style: the patched
|
||||
# ``load_reference_projects`` is called again per run by ``_project_by_id`` (``run.py:217``), and
|
||||
# a factory that rebuilt the pair would re-run ``_make_docs``' ``mkdir`` and raise. Since Step 4
|
||||
# that raise no longer surfaces — ``return_exceptions=True`` collects it into ``failures`` — so a
|
||||
# rebuilt fixture would present as "the sentinel stopped arriving", i.e. as the very semantic
|
||||
# difference this test claims to measure. The ``failures`` assertions below exist so that
|
||||
# confusion can never happen silently again.
|
||||
pair = (_road_k(tmp_path, rationale=sentinel), _bundle_kplus1(tmp_path))
|
||||
monkeypatch.setattr("portfolio_optimiser.run.load_reference_projects", lambda: pair)
|
||||
|
||||
# k=1 — two waves. The control: the cross-project fold fires and the sentinel arrives.
|
||||
seq_factory, seq_recorded = make_recording_client_factory(_ALIGNED_REPLY)
|
||||
seq = await run_portfolio(profile="local", client_factory=seq_factory, concurrency=1)
|
||||
seq_prompts = [p for p in seq_recorded if "SavingsProposal" in p]
|
||||
|
||||
assert seq.failures == (), (
|
||||
f"a project RAISED rather than running: {seq.failures}. Since Step 4 this is collected "
|
||||
"rather than propagated, so it must be excluded explicitly — otherwise a broken fixture "
|
||||
"reads as an absent sentinel, i.e. as the semantic difference this test measures"
|
||||
)
|
||||
assert seq_prompts, "no generation call happened — the fixture is broken, not the semantics"
|
||||
assert any(sentinel in p for p in seq_prompts), (
|
||||
"the sentinel did not reach k+1's hypothesis prompt at k=1 — the cross-project fold this "
|
||||
"test contrasts against is itself detached, so the k=2 absence below would prove nothing"
|
||||
)
|
||||
|
||||
# k=2 — ONE wave. The documented difference: k+1 reads the wave-start snapshot, which is empty.
|
||||
con_factory, con_recorded = make_recording_client_factory(_ALIGNED_REPLY)
|
||||
con = await run_portfolio(profile="local", client_factory=con_factory, concurrency=2)
|
||||
con_prompts = [p for p in con_recorded if "SavingsProposal" in p]
|
||||
|
||||
assert con.failures == (), (
|
||||
f"a project RAISED at k=2: {con.failures} — the sentinel's absence below would then be a "
|
||||
"crashed run, not the intra-wave semantics"
|
||||
)
|
||||
assert con_prompts, "no generation call happened at k=2"
|
||||
assert all(sentinel not in p for p in con_prompts), (
|
||||
"k's verdict reached k+1's hypothesis prompt from INSIDE the same wave — the snapshot is "
|
||||
"detached, and with it the determinism guarantee: what B sees would depend on whether A "
|
||||
"happened to finish first"
|
||||
)
|
||||
|
||||
# Both passes still ran both projects and both verdicts still land in the shared store — the
|
||||
# difference is in what each project READ, never in what the pass produced or persisted.
|
||||
assert len(seq.runs) == len(con.runs) == 2
|
||||
assert [v.id for v in con.store.verdicts] == [v.id for v in seq.store.verdicts], (
|
||||
"store CONTENT diverged between k=1 and k=2 — the intra-wave difference is supposed to be "
|
||||
"confined to read-visibility; the merge barrier must still produce identical store order"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue