fix: closed_tokens.py dispatch collisions, dead code, NBSP key, docstring + CLAUDE.md core/ status - #42
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38298b33c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR updates the top-level closed-token UCNS encoder and repository documentation to make dispatch collisions explicit (warn instead of silently dropping entries), remove dead code, clarify a NBSP whitespace key, and correct/modernize CLAUDE.md’s description of the core/ refactor status.
Changes:
- Add
warnings.warnon dispatch collisions and document the collision tie-break convention inclosed_tokens.py. - Remove unused imports/dead helper code, and de-couple class encoding/decoding from a hardcoded
32by using2 * HOST_CARRIER. - Update
WHITESPACE_TABLE’s NBSP key to"\xa0"and refreshCLAUDE.mdto reflect currentcore/implementation status.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| closed_tokens.py | Adds collision warnings + collision convention docs; removes dead code; clarifies NBSP; replaces hardcoded lattice constants with 2 * HOST_CARRIER. |
| CLAUDE.md | Updates repo structure/status text to reflect that core/ has substantive implementations in some subpackages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0573490c42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sion tests - Remove warnings.warn() from _build_dispatch_table: the known collisions (since/until/as/that) are intentional and documented in the module docstring; firing a warning unconditionally at import time breaks any -W error CI environment merely by importing the module - Remove 'import warnings' (now unused) - Add blank line after 'from __future__ import annotations' (PEP 8) - Module docstring: disk-flip sentence now says 'design invariant with no implementation yet' instead of 'implementation is in ucns_v04.py' (there is no disk-flip op in ucns_v04.py) - Tests/test_backend.py: add TestClosedTokens covering collision resolution convention for since/until/as/that
Summary
Addresses five issues across
closed_tokens.pyandCLAUDE.mdin a single atomic commit.Changes
Fix closed_tokens.py: silent class collisions in DISPATCH — conjunction/determiner senses unreachable #31 — Silent dispatch collisions:
_build_dispatch_table's inneradd()now callswarnings.warn(f"closed_tokens: dispatch collision on {token!r}; keeping first entry", stacklevel=2)instead of silently returning. Added a docstring note and module-level docstring explaining the collision convention:since/until/asresolve to the preposition sense;thatresolves to the determiner sense (by table-ordering).Fix closed_tokens.py: dead code + latent HOST_CARRIER/32 coupling #32 — Dead code + HOST_CARRIER/32 coupling: Removed unused
unit_objandmultiplyfrom theucns_v04import. Removed the_class_anchorfunction (never called). Added# reserved but unpopulatedannotation toCLASS_PUNCT_QUOTE = 12. Replaced every hardcoded32in_wrap_with_classandclass_ofwith2 * HOST_CARRIERso there is one source of truth.Fix closed_tokens.py: WHITESPACE_TABLE possible ASCII-space / NBSP key collision — needs byte verification #33 — WHITESPACE_TABLE NBSP key: Rewrote the ambiguous literal non-breaking space key as
"\xa0"with an explicit# U+00A0 NON-BREAKING SPACEcomment.Fix #36A — disk-flip docstring: Replaced "The disk-flip of an open-mark UCNS object equals the close-mark object (verified by test)." with "The disk-flip property (open-mark ↔ close-mark symmetry) is stated as a design invariant; implementation is in ucns_v04.py (TODO: add explicit test)."
Fix #36B — CLAUDE.md core/ status drift: Updated the
core/row in the Dual Package Layout table and the "What Does Not Exist Yet" section to reflect thatcore/parsing/,core/operator/, andcore/bridge/now contain substantive implementations. Status changed from "Scaffolded stubs, 0-byte files" to "Partially implemented refactor-in-progress; see individual module docstrings for what is and isn't complete."Test plan
closed_tokensand confirmwarnings.warnfires for known collisions (since,until,as,that)encode("since")returns a CLASS_PREPOSITION object (not conjunction)encode("that")returns a CLASS_DETERMINER object (not conjunction)encode("\xa0")(NBSP) encodes correctly as kind=4 whitespaceclass_of(encode("the"))round-trips correctly with the new2 * HOST_CARRIERexpressionAttributeErrorfrom removedunit_obj/multiplyimports_class_anchorremoval causes no test failures (it was never called)https://claude.ai/code/session_01HuvotBawBsg9atLV1ZThvC
Generated by Claude Code