diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index b8ad47d7..0cb5d718 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -60,4 +60,5 @@ jobs: tests/test_architectural_invariants.py \ tests/test_audio_*.py \ tests/test_pack_measurements_phase2.py \ - tests/test_register_substantive_consumption.py + tests/test_register_substantive_consumption.py \ + tests/test_speculative_subject_lifecycle.py diff --git a/core/cli_test.py b/core/cli_test.py index 4af5983e..0bd4c25a 100644 --- a/core/cli_test.py +++ b/core/cli_test.py @@ -79,6 +79,19 @@ TEST_SUITES: dict[str, tuple[str, ...]] = { # creates it, per the #136 finding that an unregistered pin runs # nowhere. ~6s, mostly two real pipeline turns per denial pair. "tests/test_negation_survives_articulation.py", + # The speculative marker must survive a neighbour's review. Teaching + # indexes a subject under its tokens, so one COHERENT correction used + # to release a token an unrelated, still-unreviewed proposal was + # relying on — serving unreviewed material with no "(speculative, not + # yet reviewed)" prefix. That is a truth-on-the-served-surface defect, + # so it belongs on the pre-push gate rather than under `full`. The file + # predates the fix but ran in NO curated suite, which is why the + # regression was invisible; it is registered here **and in smoke.yml** + # in the same PR as the fix, per the #136 finding that an unregistered + # pin runs nowhere. Registering it in both keeps the local/CI delta at + # the same ten files (H-12), so it settles nothing R-14 has to rule. + # ~9s, cache-level with two served-surface assertions. + "tests/test_speculative_subject_lifecycle.py", # ADR governance. An ADR that governs a default-ON flag records a # decision already in force, so leaving it "Proposed" makes the # governance record assert something false about the running system — diff --git a/core/cognition/pipeline.py b/core/cognition/pipeline.py index e61ceab6..ab92e450 100644 --- a/core/cognition/pipeline.py +++ b/core/cognition/pipeline.py @@ -15,7 +15,6 @@ Constraint: ChatRuntime.chat() and ChatResponse contract are unchanged. from __future__ import annotations -import hashlib import json from collections import OrderedDict @@ -35,11 +34,7 @@ from core.physics.wave_manifold import multivector_content_digest from core.reasoning.adapters import evidence_from_entailment_trace from generate.intent import classify_compound_intent from generate.intent_bridge import _is_useful_surface -from generate.intent_ratifier import ( - RatificationOutcome, - RatifiedIntent, - ratify_intent, -) +from generate.intent_ratifier import ratify_intent from generate.graph_planner import ( GraphNode, PropositionGraph, @@ -159,19 +154,27 @@ class CognitiveTurnPipeline: # Iteration order matches insertion / refresh order; lookups # remain O(1). # - # H-13 (external-assessment triage, 2026-07-28) — the value is a - # REFERENCE COUNT, not a placeholder. Seeding splits a subject - # into tokens, so two independent proposals can claim the same - # token ("wisdom" from both ``wisdom`` and ``practical wisdom``). - # While this was a flat set, promoting either one popped the - # shared token and the *other* proposal — still unreviewed — lost - # its marker, serving speculative material as though reviewed. - # A token now survives while any unpromoted proposal still claims - # it. Seeding and eviction walk the same source list, so the - # counts balance; where they disagree the count stays positive - # and the subject keeps its marker, which is the honest direction - # to fail. - self._speculative_subjects: OrderedDict[str, int] = OrderedDict() + # H-13 (external-assessment triage, 2026-07-28) — the value is the + # set of SUBJECTS CLAIMING this token, not a placeholder and not a + # count. Seeding indexes a subject under itself *and* under each + # of its ≥4-char tokens, so two independent subjects can land on + # one token ("wisdom" from both ``wisdom`` and ``practical + # wisdom``). While this was a flat set, teaching ``practical + # wisdom`` coherently popped the shared token and an unrelated, + # still-unreviewed ``wisdom`` proposal lost its marker — serving + # speculative material as though reviewed. + # + # A reference count does not fix it, and the reason is the branch + # structure at the call site: a proposal is EITHER seeded (when + # SPECULATIVE) OR released (when COHERENT), never both, so a + # release never balances a claim the same proposal made. A + # COHERENT proposal always releases claims it never held. + # Claimants make that a no-op on other subjects' tokens: a token + # dies only when the subject that actually claimed it is + # reviewed. Token derivation lives in these methods rather than + # at the call site, because ownership is exactly what the call + # site cannot see. + self._speculative_subjects: OrderedDict[str, set[str]] = OrderedDict() # ------------------------------------------------------------------ # Public API @@ -737,19 +740,19 @@ class CognitiveTurnPipeline: if proposal.epistemic_status is EpistemicStatus.SPECULATIVE: for src in sources: self._remember_speculative_subject(src) - for tok in _SUBJECT_SPLIT_RE.split(src.lower()): - if len(tok) >= 4 and tok not in _SUBJECT_STOPWORDS: - self._remember_speculative_subject(tok) elif proposal.epistemic_status is EpistemicStatus.COHERENT: # Finding 5 (audit 2026-05-20) — once teaching review - # promotes a proposal to COHERENT, the subject is no - # longer speculative; evict its tokens so the marker - # stops appearing on subsequent probes about it. + # returns COHERENT, the subject is no longer speculative; + # release its claims so the marker stops appearing on + # subsequent probes about it. + # + # H-13 — the split-and-add loop that used to live here has + # moved inside the cache. Doing it out here made every + # token its own claimant, so releasing "practical wisdom" + # also released the token "wisdom" that an unrelated, + # still-unreviewed proposal was relying on. for src in sources: self._forget_speculative_subject(src) - for tok in _SUBJECT_SPLIT_RE.split(src.lower()): - if len(tok) >= 4 and tok not in _SUBJECT_STOPWORDS: - self._forget_speculative_subject(tok) # Advance turn counter and remember surface for next correction binding. # The truth-path surface (not the served, register-decorated bytes) @@ -1039,53 +1042,76 @@ class CognitiveTurnPipeline: prompt_versor = field_state.F return ratify_intent(intent, prompt_versor, vocab=vocab) + @staticmethod + def _speculative_index_tokens(subject: str) -> tuple[str, tuple[str, ...]]: + """Return ``(owner, tokens)`` — how one subject is indexed. + + The owner is the normalized subject itself; the tokens are the + owner plus each ≥4-char non-stopword fragment, so a prefixed + parse like ``"correction: wisdom"`` still matches a probe about + ``"wisdom"``. Both the claim and the release derive their + tokens here, so the two can never disagree about what a subject + covers. + """ + owner = subject.lower().strip() + if not owner: + return "", () + tokens = [owner] + for tok in _SUBJECT_SPLIT_RE.split(owner): + if len(tok) >= 4 and tok not in _SUBJECT_STOPWORDS and tok != owner: + tokens.append(tok) + return owner, tuple(tokens) + def _remember_speculative_subject(self, subject: str) -> None: - """Claim a speculative subject token (and refresh its LRU position). + """Record ``subject`` as claiming its index tokens (refreshing LRU). Finding 5 (audit 2026-05-20). Caps the cache at ``_MAX_SPECULATIVE_SUBJECTS`` via insertion-order eviction. Empty / whitespace-only inputs are dropped silently so callers can pass raw fragments without guarding. - H-13 — increments the token's reference count rather than - re-seating a placeholder, so a token claimed by two unreviewed - proposals needs two promotions to clear. + H-13 — the subject is recorded as the *claimant* of each token it + indexes under, so a later release can tell its own tokens from a + neighbour's. """ - subject = subject.lower().strip() - if not subject: + owner, tokens = self._speculative_index_tokens(subject) + if not owner: return - count = self._speculative_subjects.get(subject, 0) - self._speculative_subjects[subject] = count + 1 - self._speculative_subjects.move_to_end(subject) + for token in tokens: + claimants = self._speculative_subjects.get(token) + if claimants is None: + self._speculative_subjects[token] = {owner} + else: + claimants.add(owner) + self._speculative_subjects.move_to_end(token) while len(self._speculative_subjects) > _MAX_SPECULATIVE_SUBJECTS: self._speculative_subjects.popitem(last=False) def _forget_speculative_subject(self, subject: str) -> None: - """Release one claim on a subject; evict it once no claim remains. + """Release ``subject``'s claims; evict each token no one still claims. - Called when a SPECULATIVE proposal is promoted to COHERENT via - the teaching review loop, so reviewed material stops being - marked speculative on later probes. No-op if the subject is - not present. + Called when teaching review returns a COHERENT proposal, so + material that has now been reviewed stops being marked + speculative on later probes. No-op for subjects that hold no + claim. - H-13 — decrements rather than pops. Token seeding is - subject-split, so two proposals can claim one token; popping - outright let a promotion strip the marker from a *different* - proposal that was still unreviewed. An unmatched release (the - promoted proposal's token set differing from what it seeded) - floors at removal and never goes negative, so it cannot borrow - against a later proposal's claim. + H-13 — releases only what *this* subject claimed. A COHERENT + proposal never seeded anything (the call site's branches are + mutually exclusive), so it holds no claim on a neighbour's + token: discarding an absent claimant leaves that token live, and + the still-unreviewed subject keeps its marker. A token is + evicted exactly when its last claimant is reviewed. """ - subject = subject.lower().strip() - if not subject: + owner, tokens = self._speculative_index_tokens(subject) + if not owner: return - count = self._speculative_subjects.get(subject) - if count is None: - return - if count <= 1: - self._speculative_subjects.pop(subject, None) - else: - self._speculative_subjects[subject] = count - 1 + for token in tokens: + claimants = self._speculative_subjects.get(token) + if claimants is None: + continue + claimants.discard(owner) + if not claimants: + self._speculative_subjects.pop(token, None) def _should_mark_speculative(self, text: str, surface: str) -> bool: """Decide whether ``surface`` should carry the SPECULATIVE marker. diff --git a/docs/assessment/31-hindrance-audit.md b/docs/assessment/31-hindrance-audit.md index 0c844948..3c7e2c0e 100644 --- a/docs/assessment/31-hindrance-audit.md +++ b/docs/assessment/31-hindrance-audit.md @@ -105,8 +105,17 @@ Ranked by leverage (cognitive/structural load removed ÷ effort), per the AGENTS **Why it hinders:** the marker is the honesty surface for unreviewed material, and the failure direction is served-unhedged — the direction this architecture treats as the expensive one. *(Provenance: surfaced by an external assessment that described this seam with the direction inverted — it claimed under-eviction; the code over-evicts. The verification that caught the inversion is what confirmed the real bug.)* **Better home:** a reference-counted cache (token → count of live proposals; decrement on promotion, evict at zero), with the LRU cap retained as a size valve. **Failing test first:** promote one proposal, assert the sibling's marker survives — red before green. **Authority:** mechanical PR + red-then-green test; flag at review whether ADR cover is wanted, since served-marker behavior changes (in the honest direction). -**Status (2026-07-28): FIXED.** `_speculative_subjects` is now `OrderedDict[str, int]` — a reference count; `_forget_speculative_subject` decrements and evicts at zero, flooring at removal so an unmatched release cannot borrow against a later proposal's claim. Four new pins in `tests/test_speculative_subject_lifecycle.py`, **all four observed red before green**; the eight pre-existing lifecycle pins pass unchanged. **Ratification note:** this changes served surfaces — markers now appear on turns where they previously vanished. The direction is toward disclosure, and it restores ADR-0021 §Articulation's stated intent rather than extending it, so it is offered as a defect fix; say if you want ADR cover before it ships. -**Found while fixing it (G-7's mechanism, live):** `tests/test_speculative_subject_lifecycle.py` — the file that pins this exact behavior — is **in no curated suite tuple**, reachable only through `full`. The pin that would have caught this defect on a pre-merge gate does not run on one. Assignment left to PR-4, which is designed to place every orphan; adding it to `TEST_SUITES["smoke"]` alone would widen N-3's ten-file delta and prejudge R-14's direction, so it was deliberately not done here. +**Status (2026-07-28): FIXED — on the second attempt, and the first attempt is the more useful record.** + +The value of `_speculative_subjects` is now the **set of subjects claiming each token** (`OrderedDict[str, set[str]]`). A token is evicted exactly when the subject that actually claimed it is reviewed. Token derivation moved from the call site into `_speculative_index_tokens`, because ownership is precisely what the call site cannot see. + +**The first fix was a reference count, and it did not work.** It rested on "seeding and eviction walk the same source list, so counts balance." They do not: the call site's arms are **mutually exclusive** — a proposal is seeded when SPECULATIVE *or* released when COHERENT, never both — so a release never balances a claim the same proposal made, and a COHERENT proposal always releases claims it never held. Refcounting therefore only repaired the sub-case where *both* proposals had been taught speculatively; the common case (an unreviewed `wisdom`, then a separate correction about `practical wisdom` that passes review on its own turn) still stripped the marker. Demonstrated after the refcount landed: `sibling marked? False`. + +The first attempt's tests passed because their helpers split tokens themselves, testing a call pattern production no longer used — **a test written to the fix instead of to the defect**. The replacement helpers call the cache exactly as `pipeline.py` §10b does, and the lead pin (`test_coherent_proposal_does_not_unmark_an_unrelated_subject`) was confirmed to fail against the refcount design before the claimant design landed. 13 pins pass, 8 of them the pre-existing lifecycle pins, unchanged. + +**Ratification note:** this changes served surfaces — markers now appear on turns where they previously vanished. The direction is toward disclosure and it restores ADR-0021 §Articulation's stated intent rather than extending it, so it is offered as a defect fix; say if you want ADR cover before it goes further. + +**Suite membership resolved, not deferred (G-7's mechanism, live).** `tests/test_speculative_subject_lifecycle.py` pinned this exact behavior while sitting **in no curated suite** — reachable only through `full` — which is why the regression was invisible. It is now registered in **both** `TEST_SUITES["smoke"]` and `smoke.yml`, per the #136 precedent that an unregistered pin runs nowhere. Registering it in both leaves the local/CI delta at **exactly ten files** (verified: local 24, CI 14, CI-only 0), so it is compatible with all three R-14 options and settles none of them. PR-4 still owns the standing ten. ## H-14 · The cognitive spine's turn method is 759 lines, with back-stamp repairs as the symptom **Verdict:** `strained` — structure debt on the highest-traffic path; H-4's sibling · **Layers:** M3/M4 diff --git a/tests/test_speculative_subject_lifecycle.py b/tests/test_speculative_subject_lifecycle.py index e6101e0d..3617f37c 100644 --- a/tests/test_speculative_subject_lifecycle.py +++ b/tests/test_speculative_subject_lifecycle.py @@ -107,63 +107,74 @@ def test_should_mark_speculative_still_iterates_correctly( # unpromoted proposal still claims it. -def _seed_proposal(pipeline: CognitiveTurnPipeline, subject: str) -> None: - """Mirror the seeding the pipeline performs for one SPECULATIVE proposal.""" +# The pipeline calls the cache once per source string and lets the cache do its +# own token indexing (H-13). These helpers mirror that exactly — a test that +# split tokens itself would be testing a call pattern production no longer uses, +# which is how the first attempt at this fix passed while the bug survived. + + +def _teach_speculative(pipeline: CognitiveTurnPipeline, subject: str) -> None: + """One SPECULATIVE proposal lands (pipeline.py 10b, SPECULATIVE arm).""" pipeline._remember_speculative_subject(subject) - for tok in subject.lower().split(): - if len(tok) >= 4: - pipeline._remember_speculative_subject(tok) -def _promote_proposal(pipeline: CognitiveTurnPipeline, subject: str) -> None: - """Mirror the eviction the pipeline performs when that proposal goes COHERENT.""" +def _teach_coherent(pipeline: CognitiveTurnPipeline, subject: str) -> None: + """One COHERENT proposal lands (pipeline.py 10b, COHERENT arm). + + Note this arm is reached by proposals that were **never seeded** — review + returns COHERENT on the same turn the correction is captured. So a release + routinely names tokens this subject never claimed. + """ pipeline._forget_speculative_subject(subject) - for tok in subject.lower().split(): - if len(tok) >= 4: - pipeline._forget_speculative_subject(tok) -def test_promoting_one_proposal_keeps_a_sibling_token_live( +def test_coherent_proposal_does_not_unmark_an_unrelated_subject( pipeline: CognitiveTurnPipeline, ) -> None: - """Promoting 'practical wisdom' must not un-mark an unreviewed 'wisdom'.""" - _seed_proposal(pipeline, "wisdom") - _seed_proposal(pipeline, "practical wisdom") + """The load-bearing case: the COHERENT proposal never seeded anything. - _promote_proposal(pipeline, "practical wisdom") + An unreviewed ``wisdom`` is outstanding. A *different* correction about + ``practical wisdom`` passes review on its own turn — it was never + SPECULATIVE, so it holds no claim on the token ``wisdom``. Releasing it + must leave the unreviewed subject marked. + """ + _teach_speculative(pipeline, "wisdom") + _teach_coherent(pipeline, "practical wisdom") assert "wisdom" in pipeline._speculative_subjects, ( - "the independent 'wisdom' proposal is still SPECULATIVE — promoting a " - "different proposal that merely shares the token must not evict it" + "a COHERENT proposal released a token it never claimed, stripping the " + "marker from an unrelated proposal that is still unreviewed" ) - # The promoted proposal's own exclusive tokens are gone. - assert "practical wisdom" not in pipeline._speculative_subjects - assert "practical" not in pipeline._speculative_subjects - - -def test_sibling_survives_at_the_served_surface( - pipeline: CognitiveTurnPipeline, -) -> None: - """The consequence that matters: the marker still fires for the sibling.""" - _seed_proposal(pipeline, "wisdom") - _seed_proposal(pipeline, "practical wisdom") - _promote_proposal(pipeline, "practical wisdom") - assert pipeline._should_mark_speculative( text="Tell me about wisdom", surface="Wisdom is the application of knowledge.", ), "unreviewed material must keep its speculative marker" -def test_token_clears_once_every_claimant_is_promoted( +def test_promoting_one_proposal_keeps_a_sibling_token_live( pipeline: CognitiveTurnPipeline, ) -> None: - """Reference counting must still reach zero — no marker that never clears.""" - _seed_proposal(pipeline, "wisdom") - _seed_proposal(pipeline, "practical wisdom") + """Both taught speculatively; reviewing one must not clear the other.""" + _teach_speculative(pipeline, "wisdom") + _teach_speculative(pipeline, "practical wisdom") - _promote_proposal(pipeline, "practical wisdom") - _promote_proposal(pipeline, "wisdom") + _teach_coherent(pipeline, "practical wisdom") + + assert "wisdom" in pipeline._speculative_subjects + # The reviewed subject's own exclusive tokens are gone. + assert "practical wisdom" not in pipeline._speculative_subjects + assert "practical" not in pipeline._speculative_subjects + + +def test_token_clears_once_its_own_claimant_is_reviewed( + pipeline: CognitiveTurnPipeline, +) -> None: + """Eviction must still happen — no marker that can never clear.""" + _teach_speculative(pipeline, "wisdom") + _teach_speculative(pipeline, "practical wisdom") + + _teach_coherent(pipeline, "practical wisdom") + _teach_coherent(pipeline, "wisdom") assert "wisdom" not in pipeline._speculative_subjects assert not pipeline._should_mark_speculative( @@ -172,37 +183,44 @@ def test_token_clears_once_every_claimant_is_promoted( ) -def test_repeated_speculative_teaching_needs_matching_promotions( +def test_claim_is_idempotent_per_subject( pipeline: CognitiveTurnPipeline, ) -> None: - """Two unreviewed proposals on one subject take two promotions to clear.""" - _seed_proposal(pipeline, "wisdom") - _seed_proposal(pipeline, "wisdom") + """Re-teaching one subject speculatively does not require two reviews. - _promote_proposal(pipeline, "wisdom") - assert "wisdom" in pipeline._speculative_subjects, ( - "one proposal reviewed, one still outstanding — still speculative" - ) + The claimant is the subject, so the same subject claiming twice is one + claim — a single review clears it. (Distinct subjects sharing a token is + the case that needs both reviewed, pinned above.) + """ + _teach_speculative(pipeline, "wisdom") + _teach_speculative(pipeline, "wisdom") - _promote_proposal(pipeline, "wisdom") + _teach_coherent(pipeline, "wisdom") assert "wisdom" not in pipeline._speculative_subjects -def test_unmatched_forget_never_underflows( +def test_release_of_an_unheld_subject_is_a_noop( pipeline: CognitiveTurnPipeline, ) -> None: - """A promotion whose seeding never happened must not push a count negative. + """Releasing a subject nobody claimed must not disturb live claims.""" + _teach_speculative(pipeline, "wisdom") - Seeding and eviction derive their token sets from the proposal object at - two different moments, so they can disagree. When they do, the safe - direction is to keep marking (a stale marker is honest-conservative; a - missing one is not) — never a negative count that would let a later - promotion strip a live sibling. - """ - pipeline._remember_speculative_subject("wisdom") - pipeline._forget_speculative_subject("wisdom") - pipeline._forget_speculative_subject("wisdom") # unmatched - pipeline._remember_speculative_subject("wisdom") + _teach_coherent(pipeline, "entirely unrelated topic") + _teach_coherent(pipeline, "practical") # shares no claim on "wisdom" - assert "wisdom" in pipeline._speculative_subjects - assert pipeline._speculative_subjects["wisdom"] == 1 + assert pipeline._speculative_subjects["wisdom"] == {"wisdom"} + + +def test_index_tokens_are_shared_by_claim_and_release( + pipeline: CognitiveTurnPipeline, +) -> None: + """Both sides derive tokens from one helper, so they cannot disagree.""" + owner, tokens = pipeline._speculative_index_tokens(" Correction: Practical WISDOM ") + assert owner == "correction: practical wisdom" + # "correction" is a subject stopword — indexing it would match every + # correction turn, so it is filtered out of the token set. + assert set(tokens) == {"correction: practical wisdom", "practical", "wisdom"} + # The owner is always indexed, even when it is shorter than the token floor. + owner_short, tokens_short = pipeline._speculative_index_tokens("ash") + assert owner_short == "ash" + assert tokens_short == ("ash",)