diff --git a/chat/runtime.py b/chat/runtime.py index 39895ef6..1cc859ec 100644 --- a/chat/runtime.py +++ b/chat/runtime.py @@ -1075,7 +1075,12 @@ class ChatRuntime: plan = plan_discourse(intent, mode, bundle) if len(plan.moves) <= 1: return None - rendered = render_plan(plan) + # Phase 2 — reflective rendering pronominalizes the focus + # subject across consecutive same-subject moves, eliminating + # the mechanical "Truth ... Truth ... Truth ..." cascade the + # Phase 1 flat renderer produced. Deterministic, replayable, + # adds no new content — purely a rendering improvement. + rendered = render_plan(plan, reflective=True) if not rendered: return None from generate.discourse_planner import FactSource diff --git a/generate/discourse_planner.py b/generate/discourse_planner.py index af012632..47ed0d73 100644 --- a/generate/discourse_planner.py +++ b/generate/discourse_planner.py @@ -777,21 +777,40 @@ def _humanize_predicate(predicate: str) -> str: return _PREDICATE_HUMANIZE.get(predicate, predicate.replace("_", " ")) -def _clause_for(move: DiscourseMove) -> str | None: +def _clause_for( + move: DiscourseMove, *, prior_focus_subject: str | None = None, +) -> str | None: """Render a single move into one declarative clause, or ``None`` when the move carries no fact (e.g. CLOSURE without summary fact). + + When ``prior_focus_subject`` is supplied AND equals ``move.fact.subject`` + byte-for-byte, the clause is emitted with ``it`` as subject instead + of repeating the lemma — the Phase 2 reflective rendering hook. + This is purely opt-in; ``render_plan(plan)`` without the + ``reflective=True`` switch never sets this argument and behaviour is + byte-identical to Phase 1. """ fact = move.fact if fact is None: return None + + subject_token = fact.subject + if ( + prior_focus_subject is not None + and fact.subject == prior_focus_subject + ): + subject_token = "it" + if move.kind is DiscourseMoveKind.ANCHOR and fact.predicate == "is_defined_as": - return f"{fact.subject} is {fact.obj}" + return f"{subject_token} is {fact.obj}" if fact.predicate == "is_defined_as": - return f"{fact.subject} is {fact.obj}" + return f"{subject_token} is {fact.obj}" if fact.predicate == "belongs_to": - return f"{fact.subject} belongs to {fact.obj}" - return f"{fact.subject} {_humanize_predicate(fact.predicate)} {fact.obj}" + return f"{subject_token} belongs to {fact.obj}" + return ( + f"{subject_token} {_humanize_predicate(fact.predicate)} {fact.obj}" + ) _MOVE_CONNECTIVE: dict[DiscourseMoveKind, str] = { @@ -803,7 +822,7 @@ _MOVE_CONNECTIVE: dict[DiscourseMoveKind, str] = { } -def render_plan(plan: DiscoursePlan) -> str: +def render_plan(plan: DiscoursePlan, *, reflective: bool = False) -> str: """Render a :class:`DiscoursePlan` as a deterministic multi-clause surface terminated with periods. @@ -814,18 +833,38 @@ def render_plan(plan: DiscoursePlan) -> str: Determinism: ``render_plan(p) == render_plan(p)`` for any plan ``p``; the function is pure. + + ``reflective`` (Phase 2 hook, opt-in): + + When ``True``, the renderer threads a tracked ``focus_subject`` + across moves: the first non-None clause sets the focus, and every + subsequent move whose ``fact.subject`` equals the current focus + is rendered with ``it`` as subject instead of repeating the lemma. + A move whose subject differs from the prior focus is treated as + a topic shift — the explicit subject is preserved and focus is + updated to the new lemma so following same-subject moves + pronominalize against the new focus. + + Default is ``False`` for back-compat with every existing call + site and test pinning the Phase-1 byte-equivalent output. The + runtime adapter (``chat.runtime._maybe_apply_discourse_planner``) + passes ``reflective=True``. """ if plan.is_empty(): return "" clauses: list[str] = [] + focus_subject: str | None = None for idx, move in enumerate(plan.moves): - clause = _clause_for(move) + prior_focus = focus_subject if reflective else None + clause = _clause_for(move, prior_focus_subject=prior_focus) if clause is None: continue if idx == 0: head = clause[0].upper() + clause[1:] if clause else clause clauses.append(f"{head}.") + if move.fact is not None: + focus_subject = move.fact.subject continue connective = _MOVE_CONNECTIVE.get(move.kind, "") if connective: @@ -834,6 +873,10 @@ def render_plan(plan: DiscoursePlan) -> str: else: head = clause[0].upper() + clause[1:] if clause else clause clauses.append(f"{head}.") + # Update focus to this move's subject so the next iteration + # can pronominalize against it (or detect topic shift). + if move.fact is not None: + focus_subject = move.fact.subject return " ".join(clauses) diff --git a/tests/test_discourse_planner_reflective.py b/tests/test_discourse_planner_reflective.py new file mode 100644 index 00000000..ecf1df3a --- /dev/null +++ b/tests/test_discourse_planner_reflective.py @@ -0,0 +1,293 @@ +"""Phase 2 — reflective rendering via subject pronominalization. + +The Phase 1 (commit ``63ffd88``) planner produces multi-clause plans +but the renderer walks moves in order without awareness of what was +just surfaced. On a typical EXPLAIN/PARAGRAPH/compound output that +yields a mechanical-feeling cascade in which the subject lemma +repeats in every clause: + + Truth is what is true. + Furthermore, truth belongs to cognition.truth. + In turn, truth grounds knowledge. + Truth belongs to epistemic.ground. + Furthermore, truth belongs to logos.core. + In turn, truth requires evidence. + +Phase 2 wires the renderer with the **lightest possible interleaved +reflection**: track the most-recently-introduced subject (the +"focus") and, when the next move's subject is byte-equal to it, +emit a pronoun ("it") instead of repeating the lemma. The +substitution rules: + +* Same subject AND no topic shift (i.e. ``move.topic`` agrees with + the prior focus) → swap to ``"it"``. +* Topic shift (TRANSITION move, or any move whose topic differs + from prior focus) → reset focus, keep the explicit subject. +* Sentence-initial position (no connective): capitalise ``"It"``. +* Mid-sentence position (after connective + comma): lowercase ``"it"``. + +This is deterministic, replayable, and adds nothing the bundle +didn't already contain — it is a pure rendering improvement. +""" + +from __future__ import annotations + +from generate.discourse_planner import ( + DiscourseMove, + DiscourseMoveKind, + DiscoursePlan, + FactSource, + GroundedFact, + render_plan, +) +from generate.intent import DialogueIntent, IntentTag, ResponseMode + + +def _fact(subject: str, predicate: str, obj: str) -> GroundedFact: + return GroundedFact( + subject=subject, + predicate=predicate, + obj=obj, + source=FactSource.PACK, + source_id="test_pack_v1", + ) + + +def _intent(subject: str = "truth") -> DialogueIntent: + return DialogueIntent(tag=IntentTag.DEFINITION, subject=subject) + + +def _move( + kind: DiscourseMoveKind, + fact: GroundedFact, + *, + given: tuple[str, ...] = (), + new: tuple[str, ...] = (), +) -> DiscourseMove: + return DiscourseMove( + kind=kind, + topic=fact.subject, + given=given, + new=new, + relation_to_previous=None, + fact=fact, + ) + + +# --------------------------------------------------------------------------- +# Pronominalization fires across consecutive same-subject moves +# --------------------------------------------------------------------------- + + +def test_reflective_replaces_repeated_subject_with_it() -> None: + """Two moves with byte-equal subject → second clause uses ``it``.""" + plan = DiscoursePlan( + intent=_intent(), + mode=ResponseMode.EXPLAIN, + moves=( + _move( + DiscourseMoveKind.ANCHOR, + _fact("truth", "is_defined_as", "what is true"), + new=("truth",), + ), + _move( + DiscourseMoveKind.SUPPORT, + _fact("truth", "belongs_to", "cognition.truth"), + given=("truth",), + new=("cognition.truth",), + ), + ), + ) + rendered = render_plan(plan, reflective=True) + assert ( + rendered + == "Truth is what is true. Furthermore, it belongs to cognition.truth." + ) + + +def test_reflective_handles_three_consecutive_same_subject_moves() -> None: + """Three same-subject moves → first is canonical, next two are ``it``.""" + plan = DiscoursePlan( + intent=_intent(), + mode=ResponseMode.PARAGRAPH, + moves=( + _move( + DiscourseMoveKind.ANCHOR, + _fact("truth", "is_defined_as", "what is true"), + new=("truth",), + ), + _move( + DiscourseMoveKind.SUPPORT, + _fact("truth", "belongs_to", "cognition.truth"), + ), + _move( + DiscourseMoveKind.RELATION, + _fact("truth", "grounds", "knowledge"), + ), + ), + ) + rendered = render_plan(plan, reflective=True) + assert rendered == ( + "Truth is what is true. " + "Furthermore, it belongs to cognition.truth. " + "In turn, it grounds knowledge." + ) + + +def test_reflective_capitalises_sentence_initial_pronoun() -> None: + """When the next clause has no connective (e.g. CLOSURE), the + pronoun must be sentence-initial and therefore capitalised.""" + plan = DiscoursePlan( + intent=_intent(), + mode=ResponseMode.PARAGRAPH, + moves=( + _move( + DiscourseMoveKind.ANCHOR, + _fact("truth", "is_defined_as", "what is true"), + ), + _move( + DiscourseMoveKind.CLOSURE, + _fact("truth", "belongs_to", "epistemic.ground"), + ), + ), + ) + rendered = render_plan(plan, reflective=True) + assert rendered == ( + "Truth is what is true. It belongs to epistemic.ground." + ) + + +def test_reflective_resets_focus_on_topic_shift() -> None: + """When the next move's subject differs (topic shift), keep the + explicit lemma — pronouns would be ambiguous.""" + plan = DiscoursePlan( + intent=_intent(), + mode=ResponseMode.PARAGRAPH, + moves=( + _move( + DiscourseMoveKind.ANCHOR, + _fact("truth", "is_defined_as", "what is true"), + ), + _move( + DiscourseMoveKind.TRANSITION, + _fact("knowledge", "belongs_to", "cognition.knowledge"), + ), + _move( + DiscourseMoveKind.RELATION, + _fact("knowledge", "grounds", "judgment"), + ), + ), + ) + rendered = render_plan(plan, reflective=True) + # After the TRANSITION shifts focus to ``knowledge``, the next + # ``knowledge`` reference becomes ``it`` again. + assert rendered == ( + "Truth is what is true. " + "Consequently, knowledge belongs to cognition.knowledge. " + "In turn, it grounds judgment." + ) + + +# --------------------------------------------------------------------------- +# Backward compatibility: reflective=False preserves Phase 1 output +# --------------------------------------------------------------------------- + + +def test_reflective_off_preserves_phase1_output() -> None: + """``reflective=False`` reproduces the pre-Phase-2 byte-equivalent + output — the default kept for any caller pinning the raw shape.""" + plan = DiscoursePlan( + intent=_intent(), + mode=ResponseMode.EXPLAIN, + moves=( + _move( + DiscourseMoveKind.ANCHOR, + _fact("truth", "is_defined_as", "what is true"), + ), + _move( + DiscourseMoveKind.SUPPORT, + _fact("truth", "belongs_to", "cognition.truth"), + ), + ), + ) + rendered = render_plan(plan, reflective=False) + assert rendered == ( + "Truth is what is true. " + "Furthermore, truth belongs to cognition.truth." + ) + + +def test_reflective_default_is_off_for_back_compat() -> None: + """``render_plan(plan)`` without an explicit kwarg keeps the + Phase-1 default so existing call sites (and existing tests) are + not silently rewritten. The runtime wiring explicitly opts in.""" + plan = DiscoursePlan( + intent=_intent(), + mode=ResponseMode.EXPLAIN, + moves=( + _move( + DiscourseMoveKind.ANCHOR, + _fact("truth", "is_defined_as", "what is true"), + ), + _move( + DiscourseMoveKind.SUPPORT, + _fact("truth", "belongs_to", "cognition.truth"), + ), + ), + ) + assert render_plan(plan) == render_plan(plan, reflective=False) + + +# --------------------------------------------------------------------------- +# Determinism: same input → same output, twice +# --------------------------------------------------------------------------- + + +def test_reflective_is_deterministic() -> None: + plan = DiscoursePlan( + intent=_intent(), + mode=ResponseMode.PARAGRAPH, + moves=( + _move( + DiscourseMoveKind.ANCHOR, + _fact("truth", "is_defined_as", "what is true"), + ), + _move( + DiscourseMoveKind.SUPPORT, + _fact("truth", "belongs_to", "cognition.truth"), + ), + _move( + DiscourseMoveKind.RELATION, + _fact("truth", "grounds", "knowledge"), + ), + ), + ) + a = render_plan(plan, reflective=True) + b = render_plan(plan, reflective=True) + assert a == b + + +# --------------------------------------------------------------------------- +# Single-move plans are byte-identical regardless of reflective mode +# (no second move ⇒ no pronoun substitution possible) +# --------------------------------------------------------------------------- + + +def test_reflective_single_move_byte_identical_to_non_reflective() -> None: + """A BRIEF/single-move plan can't trigger pronominalization — the + output must be byte-identical to ``reflective=False``. This is + the load-bearing claim that lets the cognition eval (mostly + single-fact prompts) stay byte-equal across the Phase 2 flip.""" + plan = DiscoursePlan( + intent=_intent(), + mode=ResponseMode.BRIEF, + moves=( + _move( + DiscourseMoveKind.ANCHOR, + _fact("truth", "is_defined_as", "what is true"), + ), + ), + ) + assert render_plan(plan, reflective=True) == render_plan( + plan, reflective=False + )