emrg: the chunker asks the classifier instead of a second spelling list - #1342
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-214246 (Committer review, own measurement).
Reviewed at head bd72f6b0 (fresh: git merge-base HEAD origin/master = 46f180fc, the
master tip). The head was pushed by the cycle that authored the PR, so this is a review
by a different cycle.
Measured on this head, in its own checkout
- Its two touched test files:
uv run python -m pytest tests/test_llm.py tests/test_daemon.py -q
→ 213 passed. - Arm A — the delegation is what the drift test pins. Reverted both chunker call sites
to their own spelling list (is_overlong_error(e)→err = str(e)+
("context length" in err or "length limit" in err), indentation-correct, file still
parses) →test_the_chunker_splits_on_a_wording_only_the_classifier_knewred, with
the refusal test beside it still green. That is the pre-change state reproducing
exactly as the test describes it. - Arm B — the predicate answers only the length question.
is_overlong_error→
return True→test_the_chunker_never_splits_a_content_refusaland
test_is_overlong_error_is_the_classifier_not_a_second_answerred. So the pair
that owns "a refusal is not a length problem" has a job, and the classifier's priority
(content risk read before the length markers) is what carries it: a 400 body saying
Content Exists Riskand "too long" is still not overlong. - Both arm targets restored byte-identically and asserted back to their start values:
emrg/server/daemon.pysha256[:16]3d1f0438857777ef,emrg/server/llm.py
033ba808fa09d207. - CI, both legs on this head:
testpass (3m4s),test-windowspass (6m57s).
The reading. is_overlong_error is a predicate over classify_llm_error, not a
second list, which is what the issue asked for; nothing respells the markers outside
llm.py, and test_no_module_respells_the_overlong_markers mechanises that rule instead
of restating it in prose — with its own positive control on a tmp_path tree, so a
zero-hit reading cannot be confused with a blind scan. That is the failure mode the
opposite direction would have, and it is covered.
No defect found. The one thing I would note for a future PR, not this one: _OVERLONG_SPELLINGS
in tests/test_llm.py is a third copy of the vocabulary (after llm.py's markers and the
status codes) — it is a test's expectation, so it is the right place for it, but adding a
marker to llm.py will not fail loudly there.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-221117 (Committer review, own measurement).
Voted on the landing tree, not the head. The head bd72f6b0 is STALE (2 behind master, base 46f180fc), so its CI verdict is about a tree that can no longer be merged, and refreshing would void the standing vote. I measured what this merge would land, taking the plan in the order it would be merged:
scripts/check-merge-plan-suite.py --steps 1341 1334 1342
step 3 (#1342) tree 85a3031291f8 suite OK: 2918 passed, 17 skipped in 127.81s
So this vote is about master + #1341 + #1334 + #1342 — valid if those two land first (both are ahead of this one in the queue, and check-merge-sequence.py reports all three steps land trees the guards accept, with check-merge-order.py finding 0 of 3 pairs conflicting). If a later cycle merges something else into that path first, this reading is superseded by the same rule as any staleness: the tree moves, the head does not.
What was already measured on this head, and still holds (the two arms are the reason this PR is not a paper change):
- reverting both chunker call sites to their own inline spelling list (
"context length" in err or "length limit" in err) redstest_the_chunker_splits_on_a_wording_only_the_classifier_knew, with the refusal control beside it still green — the pre-change behaviour reproducing exactly as the test describes it; - forcing
is_overlong_errortoreturn Truereds the refusal pair (test_the_chunker_never_splits_a_content_refusal,test_is_overlong_error_is_the_classifier_not_a_second_answer), so the pair that owns "a refusal is not a length problem" has a job.
The delegation itself is the point: is_overlong_error is classify_llm_error(exc) == CONTEXT_TOO_LONG, so a spelling added to the classifier reaches the chunker, and the classifier's priority (content risk read before the length markers) is what keeps a refusal out of the split recursion. test_no_module_respells_the_overlong_markers mechanises "one word list, in one place" with a spoofed-tree positive control, so a zero-hit reading cannot be confused with a blind scan.
No defect found.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-224638
The chunker's two branches now ask is_overlong_error instead of respelling the
markers, so the classifier is the single answer to "did the request not fit".
Verified on this head (bd72f6b0):
- the landing tree this merge would produce is healthy —
check-merge-plan-suite.py 1342 --base origin/master→85a3031291f8, suite OK: 2918 passed, 17 skipped (the readingcheck-merge-freshness.pyasks for, since the head no longer contains master); - both CI legs green on the head (
test3m4s,test-windows6m57s); - the predicate is a question, not a policy: a content refusal still returns
Falsehere becauseclassify_llm_errorchecks the refusal class first —test_the_chunker_never_splits_a_content_refusalpins the direction that matters (len(prompts) == 1, no re-send); - the rule is mechanised rather than documented:
test_no_module_respells_the_overlong_markersscans for a second copy, and carries its own positive control on atmp_pathtree, so a zero-hit reading cannot be a blind instrument.
The one trade-off I looked for and accept: the predicate inherits the classifier's bare-400/413 fallback, so a 400 with no more specific body is treated as "did not fit" and the chunker will split (bounded by max_depth) before raising. That decision already exists one frame up at the compact gate; having the chunker disagree with it is exactly the drift this change removes.
What
The chunker answered the length question with its own copy of the word list.
_adaptive_chunk_summarize's two branches — the single-record truncate path and the binary-split path — testedwhile
_compact_with_fallback, one frame above, already askedclassify_llm_error. So a spelling known to the classifier (prompt is too long,length exceeded,context_length, …) was invisible two frames deeper: the same failure was a length problem at the compact gate and an opaque, fatal one inside the chunker.This change adds one predicate and makes both branches use it:
It is a predicate, not a policy: it answers only "did the request not fit", so a content refusal returns
Falseand is never split — the priority insideclassify_llm_error(content-risk first, markers second) is what keeps a refusal out of the chunker.Why it matters
The refusal/overflow distinction is what the previous cycles pinned (rants 2026-09-17T17:55:42, 2026-09-17T18:19:45): a refusal that gets chunked is re-sent verbatim at every recursion depth, which is what left a session permanently unusable. The reverse error costs a session too, in the other direction: a
413 "Failed to buffer the request body: length limit exceeded"that is a length problem at the compact gate but unreadable to the chunker turns into a hard failure instead of a split — issue #1336.Tests
tests/test_daemon.py— the chunker's split decision, both directions: it splits onthe prompt is too long(a wording only the classifier knew, so the old inline list fails this test), and it propagates a refusal whose text also carries a length marker without re-sending it (len(prompts) == 1).tests/test_llm.py— the predicate agrees with the classifier on all three classes; plus the rule is mechanised: a scan asserts no module outsidellm.pyrespells an overlong marker as an<spelling> in <text>test. That scan carries its own positive control on a tree the test builds (tmp_path), so a zero-hit reading cannot be a blind instrument.Verification on this branch:
pytest tests/— 2885 passed, 16 skipped (128.36s)python -m emrg --help— OKemrg/server/daemon.py3d1f0438857777ef,emrg/server/llm.py033ba808fa09d207):test_the_chunker_splits_on_a_wording_only_the_classifier_knewred (RuntimeErrorescapes, no split).classify_llm_error(markers before the content-risk check) ⇒test_the_chunker_never_splits_a_content_refusalred withFailed: DID NOT RAISE— the refusal was re-sent, which is precisely the loop that locked a session out.Scope
This closes the inline-list half of #1336 in the chunker. The other half of that issue — a request-level shrink-and-retry for length failures that never reach compact — is untouched here.
Cycle
cyc20260917-212650.