From 63ffd8859533b67c225e5f89dbf7f2a726d0af16 Mon Sep 17 00:00:00 2001 From: Shay Date: Thu, 21 May 2026 10:06:49 -0700 Subject: [PATCH] feat(runtime): default discourse_planner=True + fast-path BRIEF short-circuit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flips ``RuntimeConfig.discourse_planner`` from ``False`` → ``True`` (the architectural intent the planner was designed for) AND adds a fast-path early return so single-fact prompts pay no extra cost. Why the flip ------------ The discourse planner apparatus has been fully wired in the codebase for some time (``generate.discourse_planner.plan_discourse`` / ``plan_compound_discourse`` / ``render_plan``, ``generate.grounding_accessors.grounding_bundle_for``, ``chat.runtime._maybe_apply_discourse_planner``) but gated off behind this flag. Investigation surfaced that: * **Cognition eval (45 cases) is byte-identical OFF vs ON** across both surface and trace_hash projections — the planner's downstream ``len(plan.moves) <= 1`` gate correctly returns ``None`` for single-fact prompts, leaving them with the exact existing pack-grounded surface. * **NARRATIVE / EXAMPLE / EXPLAIN / PARAGRAPH and compound shapes visibly lift.** ``"Tell me about memory."`` goes from a one- fragment disclosure to a 3-sentence grounded discourse. ``"What is truth, and why does it matter?"`` — currently refused as OOV because the flat classifier sees the polluted subject — becomes a 6-sentence grounded articulation via the compound bypass. * **No quality regression on existing benches.** The full bench suite (determinism / latency / speedup / versor / convergence / realizer / teaching-loop / articulation) stays 8/8 PASS with the flag on. Why the fast-path ----------------- Default-on uncovered a perf trap: the gate ran ``grounding_bundle_for(lemma)`` (pack + teaching + cross-pack queries) AND ``plan_discourse(...)`` on EVERY turn, then discarded the result when ``len(plan.moves) <= 1``. For BRIEF mode the budget ``_MODE_BUDGETS[BRIEF] = (1, 1)`` guarantees plans of length ≤ 1, so the downstream gate is guaranteed to reject — pure waste. The register matrix test runtime went from ~30s → ~14 minutes (28x slowdown) under the naive default-flip before the fast-path landed. The new short-circuit: if mode is BRIEF and not compound.is_compound(): return None skips the bundle query + plan run entirely for the common case. Compound prompts still flow through (they get auto-upgraded BRIEF → EXPLAIN on the line above). Empirical post-fast-path measurement on a 45-case eval (workers=1): OFF: 23.31s (1.93 turns/sec) ON : 17.74s (2.54 turns/sec) slowdown : 0.76x (flag-ON is actually 24% FASTER — the bundle work the OFF path also touches downstream is short-circuited cleanly when not needed) surface byte-equal: True trace_hash byte-equal: True Test updates ------------ * ``test_discourse_planner_render.py`` — invert ``test_default_runtime_config_has_flag_off`` → ``test_default_runtime_config_has_flag_on`` and rename ``test_flag_off_default_unchanged`` → ``test_flag_off_explicit_path_unchanged`` (the OFF path is still a load-bearing invariant, just no longer the default). * ``test_narrative_example_intents.py`` — three tests that assert composer-level provenance tags (``narrative-grounded``, ``example-grounded``, ``relations_chains_v1``) now explicitly set ``RuntimeConfig(discourse_planner=False)`` so they continue to exercise the underlying composer. The runtime-level multi-sentence behavior is pinned separately by ``tests/test_articulation_demo.py``. Verified -------- cognition eval (45 cases) OFF ≡ ON byte-identical pytest tests/test_discourse_planner_* 132/132 pass pytest tests/test_articulation_demo.py all claims supported pytest tests/test_narrative_example_intents.py pass pytest tests/test_runtime_config.py pass core test --suite smoke 67/67 pass core test --suite runtime 19/19 pass core test --suite packs 6/6 pass Live demo (default config): "What is knowledge?" → single sentence (BRIEF, fast-path) "Tell me about memory." → 3 grounded sentences "What is truth, and why does it matter?" → 6 grounded sentences (was: OOV) "Explain truth." → 3 grounded sentences --- chat/runtime.py | 15 ++++++++++++++ core/config.py | 16 +++++++++++++-- tests/test_discourse_planner_render.py | 21 +++++++++++++------ tests/test_narrative_example_intents.py | 27 ++++++++++++++++++++++--- 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/chat/runtime.py b/chat/runtime.py index fb719dc9..39895ef6 100644 --- a/chat/runtime.py +++ b/chat/runtime.py @@ -1023,6 +1023,21 @@ class ChatRuntime: if compound.is_compound() and mode is _ResponseMode.BRIEF: mode = _ResponseMode.EXPLAIN + # Fast path: BRIEF mode on a non-compound prompt can never + # emit > 1 move (``_MODE_BUDGETS[BRIEF] = (1, 1)``). The + # downstream ``len(plan.moves) <= 1`` gate would always + # reject — so short-circuit here, BEFORE the expensive + # ``grounding_bundle_for`` query and ``plan_discourse`` + # selector logic. This is the load-bearing perf win for + # ``discourse_planner=True`` as the runtime default; without + # it every single-fact prompt pays for a multi-source bundle + # build it can't possibly use. Confirmed empirically: + # ``tests/test_cognition_eval_register_matrix.py`` runtime + # collapsed from ~14 minutes to seconds after this gate + # landed. + if mode is _ResponseMode.BRIEF and not compound.is_compound(): + return None + # Standard gate: when upstream grounded the surface in pack or # teaching, the planner is free to engage. standard_gate = source_tag in {"pack", "teaching"} diff --git a/core/config.py b/core/config.py index 749f92dc..e8cee7dc 100644 --- a/core/config.py +++ b/core/config.py @@ -122,8 +122,20 @@ class RuntimeConfig: # and renders it as multi-clause output. Mode selection comes from # ``generate.intent.classify_response_mode``; BRIEF mode is # byte-identical to today's single-sentence pack-grounded surface - # so the default-False path is fully preserved. - discourse_planner: bool = False + # because the runtime hook (``_maybe_apply_discourse_planner``) + # returns ``None`` when the rendered plan has <= 1 move — single- + # fact prompts get exactly the same surface and trace_hash as + # the planner-off path. + # + # Default flipped to True 2026-05-21: cognition eval (45 cases) + # was verified byte-identical across both projections (surface + # AND trace_hash) flag-OFF vs flag-ON, so single-fact prompts are + # not perturbed. The flag's value shows up on NARRATIVE / EXAMPLE + # / PARAGRAPH / EXPLAIN modes and compound prompts that the flat + # classifier currently misclassifies as OOV — those turns become + # multi-clause grounded articulations rather than single-fragment + # disclosures or OOV refusals. + discourse_planner: bool = True # ADR-0068 / ADR-0069 — register pack id loaded at runtime startup. # ``None`` resolves to ``RegisterPack.unregistered()`` (the in-memory diff --git a/tests/test_discourse_planner_render.py b/tests/test_discourse_planner_render.py index 3347383e..339f7238 100644 --- a/tests/test_discourse_planner_render.py +++ b/tests/test_discourse_planner_render.py @@ -170,9 +170,15 @@ class TestRenderPlan: class TestRuntimeFlagDefault: - def test_default_runtime_config_has_flag_off(self) -> None: + def test_default_runtime_config_has_flag_on(self) -> None: + """Default flipped to True 2026-05-21 after the cognition eval + (45 cases) was confirmed byte-identical OFF vs ON across both + surface and trace_hash projections — single-fact prompts get + the same output either way; the flag only differentiates + NARRATIVE / EXAMPLE / PARAGRAPH / EXPLAIN / compound shapes. + """ cfg = RuntimeConfig() - assert cfg.discourse_planner is False + assert cfg.discourse_planner is True def test_runtime_config_field_exists(self) -> None: assert "discourse_planner" in RuntimeConfig.__dataclass_fields__ @@ -196,10 +202,13 @@ class TestRuntimeFlagOn: if "Furthermore," in response.surface or "In turn," in response.surface: assert response.surface.count(".") >= 2 - def test_flag_off_default_unchanged(self) -> None: - runtime = ChatRuntime() # default config, flag off + def test_flag_off_explicit_path_unchanged(self) -> None: + """When the operator explicitly disables the planner the surface + must remain in the existing single-sentence shape — no planner + connectives. This pins the OFF path even though the default + flipped to ON in 2026-05-21. + """ + runtime = ChatRuntime(config=RuntimeConfig(discourse_planner=False)) response = runtime.chat("Explain truth") - # Flag-off surface must remain in the existing single-sentence - # shape — no planner connectives. assert "Furthermore," not in response.surface assert "Consequently," not in response.surface diff --git a/tests/test_narrative_example_intents.py b/tests/test_narrative_example_intents.py index d0cf8c86..305e2304 100644 --- a/tests/test_narrative_example_intents.py +++ b/tests/test_narrative_example_intents.py @@ -31,6 +31,7 @@ import pytest from chat.example_surface import example_grounded_surface from chat.narrative_surface import narrative_grounded_surface from chat.runtime import ChatRuntime +from core.config import RuntimeConfig from generate.intent import IntentTag, classify_intent @@ -207,7 +208,17 @@ def test_example_max_examples_caps_output() -> None: def test_runtime_narrative_on_known_subject_routes_to_teaching() -> None: - rt = ChatRuntime() + """Composer-level invariant: NARRATIVE intent on a pack-resident + subject routes to the teaching-grounded composer and the provenance + tag ``narrative-grounded`` lands on the surface. + + Explicitly disables the discourse planner because the planner + (default-on as of 2026-05-21) intercepts on NARRATIVE shapes and + renders a multi-clause surface that drops the composer tag. The + multi-sentence runtime-level behavior is pinned separately by + ``tests/test_articulation_demo.py``. + """ + rt = ChatRuntime(config=RuntimeConfig(discourse_planner=False)) resp = rt.chat("Tell me about truth.") assert resp.grounding_source == "teaching" assert "narrative-grounded" in resp.surface @@ -228,7 +239,12 @@ def test_runtime_narrative_on_oov_routes_to_oov_invitation() -> None: def test_runtime_example_on_known_object_routes_to_teaching() -> None: - rt = ChatRuntime() + """Composer-level invariant for EXAMPLE intent on a pack-resident + object. Discourse planner explicitly disabled — see + ``test_runtime_narrative_on_known_subject_routes_to_teaching`` + for the rationale. + """ + rt = ChatRuntime(config=RuntimeConfig(discourse_planner=False)) resp = rt.chat("Give me an example of truth.") assert resp.grounding_source == "teaching" assert "example-grounded" in resp.surface @@ -242,7 +258,12 @@ def test_runtime_example_on_oov_routes_to_oov_invitation() -> None: def test_runtime_example_on_relations_object() -> None: - rt = ChatRuntime() + """Composer-level invariant — EXAMPLE intent on a relations-pack + object surfaces the relations corpus tag. Discourse planner + explicitly disabled (default-on overlays its own renderer that + drops the corpus tag). + """ + rt = ChatRuntime(config=RuntimeConfig(discourse_planner=False)) resp = rt.chat("Give me an example of parent.") assert resp.grounding_source == "teaching" assert "relations_chains_v1" in resp.surface