From f00c1583bd2d05a8a956662784d6904c739c7381 Mon Sep 17 00:00:00 2001 From: jepegit Date: Wed, 9 Sep 2026 09:01:50 +0200 Subject: [PATCH] Merge a segment too short to be one Rule SEG-01 has two ends and the planner only enforced the top one. Five of the stress corpus's segments ran under sixteen seconds and one ran seven -- a position statement and a transition, with no teaching beat in it at all. A listener does not experience that as a segment. They experience two transitions, seven seconds apart, with a sentence between them. The mirror of _split_oversized, and it rests on the same argument that function already makes: the closing recap, the emphasis marker and the spaced callbacks all land after segmentation, so a segment's finished length is only known here. Three things it has to respect. The last segment of a section is left alone. It carries the recap and the section-boundary pause, and SEG-01 exempts it for that reason. The head's transition goes with the boundary it announced -- and it is not the head's last beat, though _segment put it there, because the segment prompt and its answer were appended after it at step 8. And the new-term budget is kept rather than traded. The first version of this pushed one corpus segment past SEG-03's limit, which that rule calls an error because a planner that packs a segment could have split it; turning a pacing warning into a failed build is not a trade worth making. The test is whether the merge makes anything worse, not whether the result is inside the budget: folding seven seconds of scaffolding that teaches nothing into a segment already over the budget adds nothing to it, and refusing on the absolute count left exactly those where they were. Corpus: segments under the floor fall from twenty to twelve, and every one under thirty-one seconds is gone. Errors unchanged at two, with ten of twelve clean. Co-Authored-By: Claude Opus 5 --- src/mimem/plan/planner.py | 78 ++++++++++++++++++++++++++++++++++ tests/unit/test_plan.py | 69 +++++++++++++++++++++++++++++- tests/unit/test_script_lint.py | 19 ++++++++- 3 files changed, 163 insertions(+), 3 deletions(-) diff --git a/src/mimem/plan/planner.py b/src/mimem/plan/planner.py index e632eb3..7457cd2 100644 --- a/src/mimem/plan/planner.py +++ b/src/mimem/plan/planner.py @@ -181,6 +181,7 @@ def plan( # 13. re-check the segment sizes, and say how long the finished programme actually is. _split_oversized(ctx, script) + _merge_undersized(ctx, script) _retarget_transitions(ctx, script) _retime_orientation(ctx, script, prequestions, problem) @@ -1001,6 +1002,83 @@ def _split_oversized(ctx: _Context, script: Script) -> None: section.segments = out +def _merge_undersized(ctx: _Context, script: Script) -> None: + """Fold a segment below the duration floor into the one after it (rule SEG-01). + + The mirror of :func:`_split_oversized`, and it rests on the same argument: the closing recap, + the emphasis marker and the spaced callbacks all land after segmentation, so a segment's + finished length is only known here. What it fixes is the other end of the range. A segment + that flushed on the new-term budget with one beat in it shipped at **seven seconds** -- which + the listener does not experience as a segment but as two transitions, seven seconds apart, + with a sentence between them. + + The last segment of a section is left alone: it carries the recap and the section-boundary + pause, and rule SEG-01 exempts it for that reason. + + The new-term budget is kept, not traded. Merging two segments merges what they teach, and the + first attempt at this pushed one segment past ``SEG-03``'s limit -- which that rule calls an + error, because a planner that packs a segment could have split it. Turning a warning about + pacing into an error about working memory is not a trade worth making. + + The test is whether the merge makes anything *worse*, not whether the result is inside the + budget. The commonest short segment is a position statement and a transition with no teaching + beat at all -- seven seconds of scaffolding -- and folding that into a segment already over + the budget adds nothing to it. Refusing on the absolute count left those exactly where they + were. + """ + bounds = ctx.profile.segments + limit = ctx.profile.max_new_terms_per_segment + seen: set[str] = set() + for section in script.sections: + out: list[Segment] = [] + for segment in section.segments: + previous = out[-1] if out else None + alone = _new_terms(segment, seen) + taught = _new_terms(previous, seen) | alone + if ( + previous is not None + and previous.est_seconds < bounds.target_min + and previous.est_seconds + segment.est_seconds <= bounds.hard_max + and len(taught) <= max(limit, len(alone)) + ): + # The head's transition announced a boundary that no longer exists, so it goes. + # Not the last beat, though :func:`_segment` put it there: the segment prompt and + # its answer were appended after it, at step 8. + beats = previous.beats + boundary = next( + ( + i + for i in reversed(range(len(beats))) + if beats[i].type is BeatType.TRANSITION + ), + None, + ) + if boundary is not None: + beats = [*beats[:boundary], *beats[boundary + 1 :]] + segment.beats = [*beats, *segment.beats] + out[-1] = segment + seen |= taught + continue + if previous is not None: + seen |= _new_terms(previous, seen) + out.append(segment) + if out: + seen |= _new_terms(out[-1], seen) + section.segments = out + + +def _new_terms(segment: Segment | None, seen: set[str]) -> set[str]: + """What this segment would teach that the listener has not met yet (rule SEG-03).""" + if segment is None: + return set() + return { + concept_id + for beat in segment.beats + if beat.type in TEACHING_TYPES + for concept_id in beat.concept_ids + } - seen + + def _split( ctx: _Context, segment: Segment, title: str, hard_max: float, target_max: float ) -> list[Segment]: diff --git a/tests/unit/test_plan.py b/tests/unit/test_plan.py index 7365cc4..6c8f37e 100644 --- a/tests/unit/test_plan.py +++ b/tests/unit/test_plan.py @@ -17,7 +17,7 @@ from mimem.concepts import build as build_registry from mimem.config import Listener, Profile -from mimem.ir import BeatType, BlockKind, Document, Script +from mimem.ir import TEACHING_TYPES, BeatType, BlockKind, Document, Script, Section, Segment from mimem.lint import lint_script, script_rules from mimem.lint.rules import Severity from mimem.plan import plan @@ -375,3 +375,70 @@ def test_a_segment_prompt_is_dropped_rather_than_answered_with_a_borrowed_senten assert prompts, "the fixture should still place prompts" answers = [b for b in interphase_script.beats() if b.type is BeatType.ANSWER] assert len(answers) == len(prompts) + + +def test_no_segment_is_too_short_to_be_one( + interphase_script: Script, study_profile: Profile +) -> None: + """Rule SEG-01 has two ends, and the planner only enforced the top one. + + A segment that flushed on the new-term budget with one beat in it shipped at seven seconds -- + which a listener does not experience as a segment but as two transitions, seven seconds apart, + with a sentence between them. Five of the stress corpus's segments were under sixteen. + + The last segment of a section is exempt: it carries the recap and the section-boundary pause, + and the rule exempts it for that reason. + """ + floor = study_profile.segments.target_min + for section in interphase_script.sections: + for segment in section.segments[:-1]: + assert segment.est_seconds >= floor or _cannot_merge(segment, section, study_profile), ( + f"{segment.id} runs {segment.est_seconds:.0f}s, under the {floor:.0f}s floor" + ) + + +def _cannot_merge(segment: Segment, section: Section, profile: Profile) -> bool: + """Merging is declined when it would break the duration ceiling or the new-term budget.""" + index = section.segments.index(segment) + following = section.segments[index + 1] + combined = segment.est_seconds + following.est_seconds + terms = { + c + for s in (segment, following) + for b in s.beats + if b.type in TEACHING_TYPES + for c in b.concept_ids + } + return combined > profile.segments.hard_max or len(terms) > profile.max_new_terms_per_segment + + +def test_merging_does_not_trade_a_pacing_warning_for_a_working_memory_error( + interphase_script: Script, study_profile: Profile +) -> None: + """The first version of the merge pushed one corpus segment past SEG-03's budget. + + That rule calls it an *error* -- a planner that packs a segment could have split it -- so the + merge would have turned a warning about pacing into a failed build. + """ + from mimem.lint.script_rules import NewTermBudget + + errors = [ + v + for v in NewTermBudget(study_profile).check(interphase_script) + if v.severity is Severity.ERROR + ] + assert errors == [] + + +def test_a_merged_segment_does_not_keep_the_boundary_it_lost(interphase_script: Script) -> None: + """The head announced a boundary that no longer exists, so its transition goes with it. + + One transition per segment, not "a transition last": the segment prompt and its answer are + appended at step 8, after :func:`_segment` has put the boundary on the end, so a transition + sitting third from last is the ordinary shape. Two of them in one segment is the merge having + kept a boundary it removed. + """ + for section in interphase_script.sections: + for segment in section.segments: + transitions = [b for b in segment.beats if b.type is BeatType.TRANSITION] + assert len(transitions) <= 1, f"{segment.id} announces two boundaries" diff --git a/tests/unit/test_script_lint.py b/tests/unit/test_script_lint.py index 04dabd2..7a1a015 100644 --- a/tests/unit/test_script_lint.py +++ b/tests/unit/test_script_lint.py @@ -20,7 +20,7 @@ import pytest from mimem.config import Profile -from mimem.ir import Analogy, Anchor, Beat, BeatType, Script, Span, beat_id +from mimem.ir import TEACHING_TYPES, Analogy, Anchor, Beat, BeatType, Script, Span, beat_id from mimem.lint.script_rules import ScriptRule, script_rules Mutator = Callable[[Script], None] @@ -80,8 +80,23 @@ def _overload_new_terms(script: Script) -> None: Spread over two beats on purpose. One beat carrying five new terms is a property of the source and the rule reports it as a warning; five spread over a segment is the planner packing too much in, which it could have split. + + Which is why the segment is *chosen* rather than taken as the first one. A segment already + holding a beat with more new terms than the budget reports as a warning whatever is added to + it, so mutating that one tests nothing -- and ``segments[0]`` became such a segment the day + the planner started merging undersized ones. """ - segment = script.sections[0].segments[0] + segment = next( + ( + s + for s in script.segments() + if s.beats + and max((len(b.concept_ids) for b in s.beats if b.type in TEACHING_TYPES), default=0) + <= 2 + ), + None, + ) + assert segment is not None, "the fixture has no segment whose beats are under the budget" for i in range(2): segment.beats.append( Beat(