fix(cognition): bound speculative-subject cache + evict on COHERENT promotion (#85)
Closes audit Finding 5 (2026-05-20).
Pre-fix ``CognitiveTurnPipeline._speculative_subjects`` was a bare
``set[str]`` that only grew over a session. Two correctness gaps:
* A subject promoted to ``EpistemicStatus.COHERENT`` via the teaching
review loop kept appearing with the "(speculative, not yet
reviewed)" marker forever, contaminating reviewed material on
later probes.
* Long teaching sessions widened the per-turn substring scan in
``_should_mark_speculative`` without bound.
Fix:
* Back the cache with ``OrderedDict[str, None]`` (LRU) capped at
``_MAX_SPECULATIVE_SUBJECTS = 64``.
* Introduce ``_remember_speculative_subject`` (insert / refresh) and
``_forget_speculative_subject`` (evict) helpers; route all
SPECULATIVE inserts through them.
* When a proposal lands as ``EpistemicStatus.COHERENT``, evict the
subject and every long-enough non-stopword token derived from it,
so the marker stops appearing on reviewed material.
Iteration order in ``_should_mark_speculative`` is unchanged (keys
view); lookups remain O(1). No surface change for any case the prior
behavior didn't already mishandle, so byte-identical eval surfaces
stay stable (verified locally against ``core eval cognition`` public /
holdout / dev splits — all unchanged from MEMORY baseline).
Tests (7 new, ``tests/test_speculative_subject_lifecycle.py``):
* storage is an OrderedDict and the cap is 64
* remember normalizes (lower+strip) and drops empty input
* remember refreshes LRU position on re-insert
* cache caps at 64 with insertion-order eviction
* forget is case-insensitive and removes the entry
* forget on a missing / empty subject is a no-op
* ``_should_mark_speculative`` triggers after remember and stops
triggering after forget
Audit findings referenced:
https://github.com/AssetOverflow/core/pull/76 (Finding 5, "Unbounded
``_speculative_subjects``")
This commit is contained in:
parent
5446ab1615
commit
4f9e00a6a5
2 changed files with 159 additions and 9 deletions
|
|
@ -15,6 +15,8 @@ Constraint: ChatRuntime.chat() and ChatResponse contract are unchanged.
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from collections import OrderedDict
|
||||||
|
|
||||||
from field.state import FieldState
|
from field.state import FieldState
|
||||||
from core.cognition.result import CognitiveTurnResult
|
from core.cognition.result import CognitiveTurnResult
|
||||||
from core.cognition.surface_resolution import resolve_surface
|
from core.cognition.surface_resolution import resolve_surface
|
||||||
|
|
@ -75,6 +77,15 @@ _SUBJECT_STOPWORDS: frozenset[str] = frozenset({
|
||||||
"your", "their", "answer",
|
"your", "their", "answer",
|
||||||
})
|
})
|
||||||
|
|
||||||
|
# Finding 5 (audit 2026-05-20) — cap the speculative-subjects cache so a
|
||||||
|
# long teaching session cannot grow it without bound. 64 is large enough
|
||||||
|
# to cover every distinct teaching subject a single session realistically
|
||||||
|
# emits and small enough that the per-turn substring scan in
|
||||||
|
# ``_should_mark_speculative`` stays trivially cheap. LRU eviction: a
|
||||||
|
# subject re-encountered as SPECULATIVE refreshes its position; coherent
|
||||||
|
# promotion removes it explicitly.
|
||||||
|
_MAX_SPECULATIVE_SUBJECTS = 64
|
||||||
|
|
||||||
|
|
||||||
class CognitiveTurnPipeline:
|
class CognitiveTurnPipeline:
|
||||||
"""Thin pipeline wrapper over ChatRuntime.
|
"""Thin pipeline wrapper over ChatRuntime.
|
||||||
|
|
@ -94,7 +105,16 @@ class CognitiveTurnPipeline:
|
||||||
# (by subject substring or reflexive query shape), the surface
|
# (by subject substring or reflexive query shape), the surface
|
||||||
# is prefixed with _SPECULATIVE_SURFACE_MARKER so the user can
|
# is prefixed with _SPECULATIVE_SURFACE_MARKER so the user can
|
||||||
# tell ratified knowledge from unreviewed teaching material.
|
# tell ratified knowledge from unreviewed teaching material.
|
||||||
self._speculative_subjects: set[str] = set()
|
#
|
||||||
|
# Finding 5 (audit 2026-05-20) — backed by an OrderedDict so the
|
||||||
|
# cache is bounded (LRU, cap ``_MAX_SPECULATIVE_SUBJECTS``) and
|
||||||
|
# supports explicit eviction when a proposal is promoted to
|
||||||
|
# COHERENT. Pre-fix this was a bare ``set`` that only grew,
|
||||||
|
# which both leaked speculative markers onto reviewed subjects
|
||||||
|
# forever and widened the per-turn substring scan unboundedly.
|
||||||
|
# Iteration order matches insertion / refresh order; lookups
|
||||||
|
# remain O(1).
|
||||||
|
self._speculative_subjects: OrderedDict[str, None] = OrderedDict()
|
||||||
|
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
# Public API
|
# Public API
|
||||||
|
|
@ -204,19 +224,28 @@ class CognitiveTurnPipeline:
|
||||||
# proposal.subject (often a fragment of the correction text);
|
# proposal.subject (often a fragment of the correction text);
|
||||||
# also split-and-add each ≥4-char token so prefixed parses like
|
# also split-and-add each ≥4-char token so prefixed parses like
|
||||||
# "correction: wisdom" still match a probe about "wisdom".
|
# "correction: wisdom" still match a probe about "wisdom".
|
||||||
if proposal is not None and proposal.epistemic_status is EpistemicStatus.SPECULATIVE:
|
if proposal is not None:
|
||||||
sources: list[str] = []
|
sources: list[str] = []
|
||||||
if proposal.triple is not None and proposal.triple[0]:
|
if proposal.triple is not None and proposal.triple[0]:
|
||||||
sources.append(proposal.triple[0])
|
sources.append(proposal.triple[0])
|
||||||
if proposal.subject:
|
if proposal.subject:
|
||||||
sources.append(proposal.subject)
|
sources.append(proposal.subject)
|
||||||
for src in sources:
|
if proposal.epistemic_status is EpistemicStatus.SPECULATIVE:
|
||||||
lowered = src.lower().strip()
|
for src in sources:
|
||||||
if lowered:
|
self._remember_speculative_subject(src)
|
||||||
self._speculative_subjects.add(lowered)
|
for tok in _SUBJECT_SPLIT_RE.split(src.lower()):
|
||||||
for tok in _SUBJECT_SPLIT_RE.split(src.lower()):
|
if len(tok) >= 4 and tok not in _SUBJECT_STOPWORDS:
|
||||||
if len(tok) >= 4 and tok not in _SUBJECT_STOPWORDS:
|
self._remember_speculative_subject(tok)
|
||||||
self._speculative_subjects.add(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.
|
||||||
|
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
|
# Advance turn counter and remember surface for next correction binding
|
||||||
self._turn_number += 1
|
self._turn_number += 1
|
||||||
|
|
@ -348,6 +377,34 @@ class CognitiveTurnPipeline:
|
||||||
)
|
)
|
||||||
return ratify_intent(intent, prompt_versor, vocab=vocab)
|
return ratify_intent(intent, prompt_versor, vocab=vocab)
|
||||||
|
|
||||||
|
def _remember_speculative_subject(self, subject: str) -> None:
|
||||||
|
"""Add (or refresh LRU position of) a speculative subject token.
|
||||||
|
|
||||||
|
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.
|
||||||
|
"""
|
||||||
|
subject = subject.lower().strip()
|
||||||
|
if not subject:
|
||||||
|
return
|
||||||
|
self._speculative_subjects.pop(subject, None)
|
||||||
|
self._speculative_subjects[subject] = None
|
||||||
|
while len(self._speculative_subjects) > _MAX_SPECULATIVE_SUBJECTS:
|
||||||
|
self._speculative_subjects.popitem(last=False)
|
||||||
|
|
||||||
|
def _forget_speculative_subject(self, subject: str) -> None:
|
||||||
|
"""Evict a subject from the speculative-marker cache.
|
||||||
|
|
||||||
|
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.
|
||||||
|
"""
|
||||||
|
subject = subject.lower().strip()
|
||||||
|
if subject:
|
||||||
|
self._speculative_subjects.pop(subject, None)
|
||||||
|
|
||||||
def _should_mark_speculative(self, text: str, surface: str) -> bool:
|
def _should_mark_speculative(self, text: str, surface: str) -> bool:
|
||||||
"""Decide whether ``surface`` should carry the SPECULATIVE marker.
|
"""Decide whether ``surface`` should carry the SPECULATIVE marker.
|
||||||
|
|
||||||
|
|
|
||||||
93
tests/test_speculative_subject_lifecycle.py
Normal file
93
tests/test_speculative_subject_lifecycle.py
Normal file
|
|
@ -0,0 +1,93 @@
|
||||||
|
"""Speculative-subject cache lifecycle (Finding 5, audit 2026-05-20).
|
||||||
|
|
||||||
|
Pre-fix ``CognitiveTurnPipeline._speculative_subjects`` was an unbounded
|
||||||
|
``set[str]`` that only grew over a session. Two correctness gaps:
|
||||||
|
|
||||||
|
* A subject promoted to ``EpistemicStatus.COHERENT`` via the teaching
|
||||||
|
review loop kept appearing with the "(speculative, not yet reviewed)"
|
||||||
|
marker forever.
|
||||||
|
* Long teaching sessions widened the per-turn substring scan inside
|
||||||
|
``_should_mark_speculative`` without bound.
|
||||||
|
|
||||||
|
These tests pin the lifecycle so the fix cannot silently regress.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from collections import OrderedDict
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from chat.runtime import ChatRuntime
|
||||||
|
from core.cognition import CognitiveTurnPipeline
|
||||||
|
from core.cognition.pipeline import _MAX_SPECULATIVE_SUBJECTS
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture()
|
||||||
|
def pipeline() -> CognitiveTurnPipeline:
|
||||||
|
return CognitiveTurnPipeline(runtime=ChatRuntime())
|
||||||
|
|
||||||
|
|
||||||
|
def test_storage_is_bounded_ordereddict(pipeline: CognitiveTurnPipeline) -> None:
|
||||||
|
assert isinstance(pipeline._speculative_subjects, OrderedDict)
|
||||||
|
assert _MAX_SPECULATIVE_SUBJECTS == 64
|
||||||
|
|
||||||
|
|
||||||
|
def test_remember_inserts_and_normalizes(pipeline: CognitiveTurnPipeline) -> None:
|
||||||
|
pipeline._remember_speculative_subject(" TRUTH ")
|
||||||
|
assert "truth" in pipeline._speculative_subjects
|
||||||
|
pipeline._remember_speculative_subject("")
|
||||||
|
pipeline._remember_speculative_subject(" ")
|
||||||
|
# Empty / whitespace inputs are silently dropped
|
||||||
|
assert list(pipeline._speculative_subjects) == ["truth"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_remember_refreshes_lru_position(pipeline: CognitiveTurnPipeline) -> None:
|
||||||
|
pipeline._remember_speculative_subject("alpha")
|
||||||
|
pipeline._remember_speculative_subject("beta")
|
||||||
|
pipeline._remember_speculative_subject("alpha")
|
||||||
|
# alpha was re-touched after beta, so alpha is now the MRU entry
|
||||||
|
assert list(pipeline._speculative_subjects) == ["beta", "alpha"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_cache_caps_at_max(pipeline: CognitiveTurnPipeline) -> None:
|
||||||
|
for i in range(_MAX_SPECULATIVE_SUBJECTS + 10):
|
||||||
|
pipeline._remember_speculative_subject(f"subj{i:03d}")
|
||||||
|
assert len(pipeline._speculative_subjects) == _MAX_SPECULATIVE_SUBJECTS
|
||||||
|
# Oldest 10 entries (subj000..subj009) evicted; newest survive
|
||||||
|
keys = list(pipeline._speculative_subjects)
|
||||||
|
assert "subj000" not in keys
|
||||||
|
assert "subj009" not in keys
|
||||||
|
assert "subj010" in keys
|
||||||
|
assert keys[-1] == f"subj{_MAX_SPECULATIVE_SUBJECTS + 9:03d}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_forget_removes(pipeline: CognitiveTurnPipeline) -> None:
|
||||||
|
pipeline._remember_speculative_subject("truth")
|
||||||
|
pipeline._remember_speculative_subject("light")
|
||||||
|
pipeline._forget_speculative_subject("Truth") # case-insensitive
|
||||||
|
assert "truth" not in pipeline._speculative_subjects
|
||||||
|
assert "light" in pipeline._speculative_subjects
|
||||||
|
|
||||||
|
|
||||||
|
def test_forget_missing_is_noop(pipeline: CognitiveTurnPipeline) -> None:
|
||||||
|
pipeline._remember_speculative_subject("truth")
|
||||||
|
pipeline._forget_speculative_subject("never-added")
|
||||||
|
pipeline._forget_speculative_subject("")
|
||||||
|
assert list(pipeline._speculative_subjects) == ["truth"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_should_mark_speculative_still_iterates_correctly(
|
||||||
|
pipeline: CognitiveTurnPipeline,
|
||||||
|
) -> None:
|
||||||
|
"""The marker decision still triggers on stored subjects."""
|
||||||
|
pipeline._remember_speculative_subject("wisdom")
|
||||||
|
assert pipeline._should_mark_speculative(
|
||||||
|
text="Tell me about wisdom",
|
||||||
|
surface="Wisdom is the application of knowledge.",
|
||||||
|
)
|
||||||
|
pipeline._forget_speculative_subject("wisdom")
|
||||||
|
assert not pipeline._should_mark_speculative(
|
||||||
|
text="Tell me about wisdom",
|
||||||
|
surface="Wisdom is the application of knowledge.",
|
||||||
|
)
|
||||||
Loading…
Reference in a new issue