fix(runtime): config-replace + thin API wrappers + stale docstring
Three independent hygiene fixes named in the 2026-05-19 design review.
All small, all observable, none architectural.
1. ``RuntimeConfig`` flag drop on pack_id / frame_pack override
chat/runtime.py:306-320 used to enumerate fields by hand when
reconstructing RuntimeConfig under the pack_id / frame_pack
override path. The list stopped at ``admissibility_margin`` and
silently dropped FIVE newer flags: identity_pack, ethics_pack,
forward_graph_constraint, composed_surface, thread_anaphora.
Caller side-effect:
ChatRuntime(pack_id="x", config=RuntimeConfig(composed_surface=True))
.config.composed_surface == False # silently lost
Fix: ``dataclasses.replace(config, input_packs=..., frame_pack=...)``.
Every field on the dataclass survives by construction; future
additions never need a synchronized edit on this path.
2. Stale CAUSE / VERIFICATION docstring
tests/test_intent_classification_extensions.py described a sixth
runtime-side fix (pack_grounded_surface fallback for
CAUSE/VERIFICATION) that was considered, reverted, and the file's
own test classes pin the opposite contract. Docstring now states
the doctrine correctly: no fallback, deliberately, so the discovery
layer can log the teaching-gap signal.
3. Thin convenience wrappers: respond / achat / arespond
tests/test_achat.py and tests/test_language_pack_runtime.py
referenced these public methods since 2026-05-14, but they were
never implemented on ChatRuntime — those 12 tests had been red on
every full-lane run since the rebase. Added as thin wrappers:
respond(text) -> ChatResponse.surface
achat(text) -> async wrapper around chat()
arespond(text)-> async wrapper around respond()
The async wrappers are deliberately NOT genuinely non-blocking —
the underlying CPU-bound walk/recall/composition remains sync.
Docstrings say so explicitly. Callers needing real concurrency
should wrap in asyncio.to_thread at the call site; promoting the
wrappers to true async event-loop integration is a future change
gated by an actual concurrent caller.
Regression coverage:
tests/test_runtime_config_passthrough.py — 4 tests
- all 19 RuntimeConfig fields survive a pack_id override
- all five newer flags survive a frame_pack override
- no-override path preserves caller config by identity (no rebuild)
- the four public methods exist and are callable
Verification:
44/44 affected tests green (was 12 red pre-fix).
Cognition eval byte-identical on both splits.
No surface-format change; this commit is pure plumbing.
This commit is contained in:
parent
a084f1db21
commit
c6b4f1d21e
3 changed files with 152 additions and 18 deletions
|
|
@ -305,21 +305,19 @@ class ChatRuntime:
|
|||
) -> None:
|
||||
if pack_id is not None or frame_pack is not None:
|
||||
pack_ids = (pack_id,) if isinstance(pack_id, str) else tuple(pack_id or config.input_packs)
|
||||
resolved_config = RuntimeConfig(
|
||||
# Use dataclasses.replace so newer RuntimeConfig fields
|
||||
# (identity_pack, ethics_pack, forward_graph_constraint,
|
||||
# composed_surface, thread_anaphora, etc.) survive the
|
||||
# pack_id / frame_pack override path. The previous manual
|
||||
# reconstruction silently dropped any field not enumerated
|
||||
# here, which would let a caller like
|
||||
# ``ChatRuntime(pack_id="x", config=RuntimeConfig(composed_surface=True))``
|
||||
# lose composed_surface without warning.
|
||||
from dataclasses import replace as _dc_replace
|
||||
resolved_config = _dc_replace(
|
||||
config,
|
||||
input_packs=pack_ids,
|
||||
output_language=config.output_language,
|
||||
frame_pack=frame_pack or config.frame_pack,
|
||||
max_tokens=config.max_tokens,
|
||||
allow_cross_language_recall=config.allow_cross_language_recall,
|
||||
allow_cross_language_generation=config.allow_cross_language_generation,
|
||||
vault_reproject_interval=config.vault_reproject_interval,
|
||||
use_salience=config.use_salience,
|
||||
salience_top_k=config.salience_top_k,
|
||||
inhibition_threshold=config.inhibition_threshold,
|
||||
inner_loop_admissibility=config.inner_loop_admissibility,
|
||||
admissibility_threshold=config.admissibility_threshold,
|
||||
admissibility_mode=config.admissibility_mode,
|
||||
admissibility_margin=config.admissibility_margin,
|
||||
)
|
||||
else:
|
||||
resolved_config = config
|
||||
|
|
@ -1107,6 +1105,35 @@ class ChatRuntime:
|
|||
def _unknown_domain_response(self, field_state: FieldState, filtered: list[str]) -> ChatResponse:
|
||||
return self._stub_response(field_state)
|
||||
|
||||
def respond(self, text: str, max_tokens: int | None = None) -> str:
|
||||
"""Return only the user-facing surface string for *text*.
|
||||
|
||||
Convenience wrapper around :meth:`chat` for callers that need
|
||||
the raw surface without ChatResponse provenance — REPLs, simple
|
||||
scripts, and the existing test_language_pack_runtime suite.
|
||||
For audit / telemetry / verdict access, call :meth:`chat`.
|
||||
"""
|
||||
return self.chat(text, max_tokens=max_tokens).surface
|
||||
|
||||
async def achat(self, text: str, max_tokens: int | None = None) -> ChatResponse:
|
||||
"""Async-compatible convenience wrapper around :meth:`chat`.
|
||||
|
||||
This is a thin async surface; the underlying call is still
|
||||
synchronous CPU-bound work (versor walk, vault recall, surface
|
||||
composition). Use this only for integration with asyncio-based
|
||||
callers that need an awaitable. No real off-thread execution
|
||||
is performed — if true non-blocking concurrency is required,
|
||||
wrap calls in :func:`asyncio.to_thread` at the call site.
|
||||
"""
|
||||
return self.chat(text, max_tokens=max_tokens)
|
||||
|
||||
async def arespond(self, text: str, max_tokens: int | None = None) -> str:
|
||||
"""Async-compatible convenience wrapper around :meth:`respond`.
|
||||
|
||||
Same caveats as :meth:`achat` — wrapper, not true async.
|
||||
"""
|
||||
return self.respond(text, max_tokens=max_tokens)
|
||||
|
||||
def correct(self, text: str, target_turn: int = -1, max_tokens: int | None = None) -> ChatResponse:
|
||||
tokens = self._tokenize(text)
|
||||
filtered = self._apply_oov_policy(tokens)
|
||||
|
|
|
|||
|
|
@ -27,11 +27,12 @@ Five gaps pinned by this file:
|
|||
produces/induces/yields) now routes to
|
||||
CAUSE with X as subject.
|
||||
|
||||
Plus a sixth runtime-side fix: CAUSE / VERIFICATION intents now fall
|
||||
through to ``pack_grounded_surface`` when no teaching chain or cross-
|
||||
pack chain is rooted on the subject lemma. Honest fallback — the
|
||||
surface explicitly tags the pack source and emits no fabricated
|
||||
causal claim.
|
||||
A sixth runtime-side decision was *considered* (CAUSE / VERIFICATION
|
||||
falling through to ``pack_grounded_surface`` when no teaching chain
|
||||
exists) but deliberately not adopted. Doing so would mask the
|
||||
teaching-gap signal the discovery layer uses to identify chains worth
|
||||
authoring — see ``tests/test_discovery_candidates``. The
|
||||
``TestCauseVerificationNoPackFallback`` class below pins this doctrine.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
|
|||
106
tests/test_runtime_config_passthrough.py
Normal file
106
tests/test_runtime_config_passthrough.py
Normal file
|
|
@ -0,0 +1,106 @@
|
|||
"""Regression test for ChatRuntime.__init__ config-flag passthrough.
|
||||
|
||||
Prior to fix d-2026-05-19, ``ChatRuntime.__init__`` manually reconstructed
|
||||
``RuntimeConfig`` field-by-field whenever ``pack_id`` or ``frame_pack``
|
||||
was supplied. The manual list was incomplete: newer flags
|
||||
(``identity_pack``, ``ethics_pack``, ``forward_graph_constraint``,
|
||||
``composed_surface``, ``thread_anaphora``) were silently dropped, so a
|
||||
caller like
|
||||
|
||||
ChatRuntime(pack_id="x", config=RuntimeConfig(composed_surface=True))
|
||||
|
||||
would lose ``composed_surface=True`` without warning.
|
||||
|
||||
The fix uses ``dataclasses.replace`` so every field on the dataclass
|
||||
survives by construction. This test pins that contract: pass a config
|
||||
with all current flags set to *non-default* values, instantiate
|
||||
``ChatRuntime`` with a ``pack_id`` override, and assert every flag
|
||||
survived on ``runtime.config``.
|
||||
|
||||
If a new flag is added to ``RuntimeConfig``, this test should catch any
|
||||
future drift on the override path provided the default differs from
|
||||
the test value (the assertion compares post-init value against the
|
||||
explicitly-set value, not against the default).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import fields
|
||||
|
||||
from chat.runtime import ChatRuntime
|
||||
from core.config import RuntimeConfig
|
||||
|
||||
|
||||
def test_all_runtime_config_flags_survive_pack_id_override() -> None:
|
||||
custom = RuntimeConfig(
|
||||
input_packs=("en_minimal_v1",),
|
||||
output_language="en",
|
||||
frame_pack="en",
|
||||
max_tokens=17, # non-default
|
||||
allow_cross_language_recall=False,
|
||||
allow_cross_language_generation=True,
|
||||
vault_reproject_interval=99,
|
||||
use_salience=False,
|
||||
salience_top_k=8,
|
||||
inhibition_threshold=0.42,
|
||||
inner_loop_admissibility=True,
|
||||
admissibility_threshold=0.13,
|
||||
admissibility_mode="margin",
|
||||
admissibility_margin=0.7,
|
||||
identity_pack="precision_first_v1",
|
||||
ethics_pack="default_general_ethics_v1",
|
||||
forward_graph_constraint=True,
|
||||
composed_surface=True,
|
||||
thread_anaphora=True,
|
||||
)
|
||||
|
||||
runtime = ChatRuntime(pack_id="en_minimal_v1", config=custom)
|
||||
|
||||
for field in fields(RuntimeConfig):
|
||||
if field.name == "input_packs":
|
||||
# The pack_id override deliberately rewrites input_packs;
|
||||
# other fields must survive verbatim.
|
||||
continue
|
||||
expected = getattr(custom, field.name)
|
||||
actual = getattr(runtime.config, field.name)
|
||||
assert actual == expected, (
|
||||
f"RuntimeConfig.{field.name} did not survive pack_id override: "
|
||||
f"expected {expected!r}, got {actual!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_all_runtime_config_flags_survive_frame_pack_override() -> None:
|
||||
custom = RuntimeConfig(
|
||||
composed_surface=True,
|
||||
thread_anaphora=True,
|
||||
forward_graph_constraint=True,
|
||||
identity_pack="precision_first_v1",
|
||||
ethics_pack="default_general_ethics_v1",
|
||||
)
|
||||
|
||||
runtime = ChatRuntime(frame_pack="en", config=custom)
|
||||
|
||||
assert runtime.config.composed_surface is True
|
||||
assert runtime.config.thread_anaphora is True
|
||||
assert runtime.config.forward_graph_constraint is True
|
||||
assert runtime.config.identity_pack == "precision_first_v1"
|
||||
assert runtime.config.ethics_pack == "default_general_ethics_v1"
|
||||
|
||||
|
||||
def test_no_override_path_passes_config_through_unchanged() -> None:
|
||||
"""When neither pack_id nor frame_pack is supplied, the user's
|
||||
config is preserved verbatim (it is the same object identity)."""
|
||||
custom = RuntimeConfig(composed_surface=True)
|
||||
runtime = ChatRuntime(config=custom)
|
||||
# Same object: no reconstruction happened.
|
||||
assert runtime.config is custom
|
||||
assert runtime.config.composed_surface is True
|
||||
|
||||
|
||||
def test_chat_method_exists() -> None:
|
||||
"""Smoke: the user-facing public methods are present."""
|
||||
rt = ChatRuntime()
|
||||
assert callable(getattr(rt, "chat", None))
|
||||
assert callable(getattr(rt, "respond", None))
|
||||
assert callable(getattr(rt, "achat", None))
|
||||
assert callable(getattr(rt, "arespond", None))
|
||||
Loading…
Reference in a new issue