Decouple the chained-emitter tests from the shipped particle vocabulary - #378
Merged
Conversation
The _group PARTICLE_OR_GIVEN tests borrowed their precondition from the shipped vocabulary, which coupled them to #360's curation. Two things turned out to be wrong with that, both measured against "Freiherr von Richthofen": The guard asserted the wrong set. It required a lead word in `titles & particles_ambiguous`; the emitter requires `titles & PARTICLES`: freiherr out of particles_ambiguous only -> still chains freiherr out of particles entirely -> no chain freiherr out of titles only -> no chain The two intersections are the same three words today, which is why it read as correct. And #360 cannot empty the right one: it moves words between the may-be-given and never-given halves, both subsets of particles, so particle membership survives either way. The coupling was unnecessary regardless. Reachability is a property of the emitter -- a caller may configure a titles/particles overlap themselves, with no invariant against it and no warning -- so an empty shipped intersection would leave the emitter reachable and merely unexercised by default. The old guard's advice to "remove the emitter rather than repointing them" would have deleted live code. Tests now supply the memberships. The new reachability test carries two controls, one per role: a plain-title lead (no chain, _assign reports instead) and an unambiguous chained word (chain fires, nothing to report) -- so a pass cannot be the grouping it would have produced anyway. Verified: normal 91 passed shipped overlap emptied 91 passed (decoupled) emitter suppressed 6 failed (still sensitive) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #378 +/- ##
=======================================
Coverage 98.51% 98.51%
=======================================
Files 44 44
Lines 2895 2895
=======================================
Hits 2852 2852
Misses 43 43 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Review of the previous commit found it half-done, and found the comment block claiming otherwise. The emitter asks two things of two different words. The leading word needs titles & particles; the chained word needs particles_ambiguous. _overlap_parser supplied only the first, so the tests stayed coupled to #360 through 'von'/'van' -- moving those to the never-given half failed seven of them, which is the exact breakage the commit set out to prevent. Constants are now split by role and both halves are supplied. Three claims in that block were also wrong: "drop it from either and there is no chain" -- false for the titles half. Measured, freiherr out of titles: family='von Richthofen', so the chain is intact; what is lost is the REPORT, because the merge sits outside the all(title(x)) guard, and _assign then reports a fork about 'Freiherr' instead. the new test's docstring opened with "both a title and an ambiguous particle" -- verbatim the wrong set the block twenty lines above exists to retire. _overlap_parser's rationale cited 'Do St Johnson', which no test in the file parses; the string lives only in comments. The fixture repeated the docstring's error: it added the leading word to all three sets, so nothing could contradict the wrong precondition. Measured, that third membership is inert -- titles+particles and titles+particles+ambiguous parse byte-identically. Dropped, and control 3 now pins it, which makes the correction executable rather than prose. test_properties.py carried the same borrow on a bigger surface: its 'Freiherr ' lead is one of four in a sweep over every ambiguous particle, and if that lead goes transparent the _group shapes leave the sweep while it keeps passing. Supplied there too. normal 2486 passed leading word out of titles+particles only the cases.py tripwire chained words -> never-given no test in this diff emitter suppressed 8 failed (still sensitive) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Groundwork for #360, which we didn't want blocked on a vocabulary/test coupling. It turned out the coupling was never real, and the guard that enforced it was asserting the wrong set.
Two commits: the sweep, and the fixes from a review that found the sweep half-done. Both are described below.
The guard asserted the wrong intersection
test_the_chained_emitter_is_still_reachablerequired a lead word intitles & particles_ambiguous. What the emitter actually requires, measured against"Freiherr von Richthofen"one membership at a time:von, from_groupfreiherrout ofparticlesgiven='von'von, from_assignfreiherrout oftitlesfamily='von Richthofen'Freiherr, from_assignvonout ofparticles_ambiguousSo the two memberships buy different things:
particlesis what produces the chain,titlesis what the emitter'sall(title(x) for x in range(k))guard needs to report it — themerge()sits outside that guard. Both intersections are['do','freiherr','st']today, which is why the wrong set read as correct.#360 cannot empty the right one. Curation moves words between the may-be-given and never-given halves; both are subsets of
particles, so particle membership survives either way. The emitter was never at risk.The coupling was unnecessary anyway
Reachability is a property of the emitter, not of the shipped word lists. A caller can configure the overlap themselves —
Lexiconasserts no invariant against it and construction warns about nothing:An empty shipped intersection would leave the emitter reachable and merely unexercised by default. The old guard's remediation advice — "remove the emitter rather than repointing them" — would have deleted live code on a false signal.
What changed
The emitter tests supply their memberships instead of borrowing them, split by the role each word plays:
_TITLE_PARTICLES— the leading words, giventitles+particles_CHAINED_PARTICLES— the chained words (von,van), givenparticles_ambiguoustest_the_chained_emitter_is_reachable_by_constructioncarries three controls, one per membership the claim rests on: drop the leading word'sparticlesand the chain goes; drop the chained word'sparticles_ambiguousand the report goes; drop the leading word'sparticles_ambiguousand nothing moves. That third one is the correction made executable — it is the membership the old guard asserted, and it buys the emitter nothing.tests/v2/test_properties.pycarried the same borrow on a larger surface: its"Freiherr "lead feeds a sweep over every ambiguous particle, and if that lead goes transparent the_groupshapes leave the sweep while it keeps passing. Supplied there too.Shipped-vocabulary behavior stays pinned where it belongs —
tests/v2/cases.py'sFreiherr von Richthofenrow. Note that row tracks the parse, not the memberships: movingfreiherrbetween the particle halves leaves it green, which is correct.Review round
A three-agent review (test coverage, comment accuracy, general) found the first commit half-done, and found the comment block claiming otherwise. All fixed in the second commit:
von/vancame from the shipped lexicon, so moving them to never-given failed seven of these tests, the exact breakage the commit set out to preventtitleshalf (see the table above)_overlap_parser's rationale cited'Do St Johnson', which no test in the file parsesVerification
tests/v2/test_parser.pytitles+particlescases.pytripwire fires_group's emitter suppressedEvery simulation asserts it took effect before running, so none is a no-op measurement.
ruff checkandmypyclean.Two tests outside this diff still borrow
van's ambiguity —test_parsedname_repr_includes_ambiguities_lineandtest_leading_particle_detail_names_the_role_it_got. Left alone deliberately: they fail loudly with an obvious fix (repoint to another ambiguous particle), which is not the failure mode this PR is about.Groundwork for #360.
🤖 Generated with Claude Code