Add closed-token UCNS encoder and test suite - #17
Conversation
Introduces edcmbone.ucns.closed_tokens — a UCNS encoder that maps the 16 closed classes of English (pronouns, determiners, prepositions, conjunctions, auxiliaries, particles, interjections, whitespace, punctuation, and numerals) to UCNSObjects on a 32-gonal lattice. Adds the companion test suite (12 tests) in Tests/test_closed_tokens.py. Depends on ucns_v04 (to be added separately). https://claude.ai/code/session_01CnFjpBzuNFY5iShsD8dqpn
|
I checked the trigger and PR context, and there isn’t an actionable request yet—just agent mentions ( So I did not make any code changes, commit, or open a follow-up PR. Once you share a concrete instruction (e.g., fix failing CI, address a review comment, add a test), I can execute it end-to-end. |
![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: 5340e46342
ℹ️ 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".
| from ucns_v04 import ( | ||
| UCNSObject, AnchorPayload, | ||
| unit_obj, multiply, | ||
| ) |
There was a problem hiding this comment.
This module hard-imports ucns_v04, but that module is not present anywhere in this repository or package metadata, so importing edcmbone.ucns immediately raises ModuleNotFoundError and makes the new encoder unusable in a clean checkout. As introduced, both runtime usage and the added test file fail during import unless callers manually provide an external module.
Useful? React with 👍 / 👎.
|
|
||
| from fractions import Fraction | ||
| from ucns_v04 import unit_obj, multiply | ||
| from closed_tokens import ( |
There was a problem hiding this comment.
The test imports closed_tokens as a top-level module, but this commit adds it under backend/src/edcmbone/ucns/closed_tokens.py and no top-level closed_tokens.py exists. In a standard test run (repo root or installed package), this import cannot resolve, so the suite fails at collection even before assertions run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a closed-token UCNS encoder to the edcmbone backend, mapping English closed-class tokens (plus whitespace, punctuation, and numerals) to UCNS objects, and introduces a dedicated test suite to validate encoding properties.
Changes:
- Added
closed_tokens.pyimplementing a dispatch-table-driven UCNS encoding for closed tokens with class/payload structure. - Added
edcmbone.ucnspackage initializer exporting the public API (encode,class_of,feature_payload_of,DISPATCH,CLASS_NAMES). - Added
Tests/test_closed_tokens.pywith a suite of tests validating coverage, uniqueness, and feature distinctions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| backend/src/edcmbone/ucns/closed_tokens.py | Implements the closed-token encoder, tables, and class/payload helpers. |
| backend/src/edcmbone/ucns/init.py | Exposes the UCNS closed-token API at the package level. |
| Tests/test_closed_tokens.py | Adds tests for encoding, class recovery, uniqueness, and basic feature differentiation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| and small numerals to UCNS objects on a 16-gonal host carrier. | ||
|
|
||
| Class partition (host anchor index → class): | ||
| 0: pronoun | ||
| 1: determiner | ||
| 2: preposition | ||
| 3: conjunction | ||
| 4: auxiliary verb | ||
| 5: particle / catch-all | ||
| 6: interjection | ||
| 7: whitespace | ||
| 8: punctuation — sentence terminal | ||
| 9: punctuation — internal juncture | ||
| 10: punctuation — pairing open | ||
| 11: punctuation — pairing close | ||
| 12: punctuation — quote (smart open/close handled in pairing classes) | ||
| 13: punctuation — affix / connective | ||
| 14: punctuation — modal (ellipsis) | ||
| 15: numeral | ||
|
|
||
| Each token encodes as a UCNSObject with: | ||
| - host length 1 (a single anchor at the class's position on the 16-gonal lattice) | ||
| - payload encoding the per-class features (person/number/case for pronouns, etc.) | ||
| - face bit unused at the host level (set to 0) |
There was a problem hiding this comment.
The module docstring describes a "16-gonal host carrier" and "host length 1", but the actual encoding in _wrap_with_class() constructs a normalized 2-anchor object on a 32-gonal lattice with a structural marker at theta=0. This mismatch will confuse readers and future maintainers; please update the top-level documentation to match the current encoding scheme (or adjust the implementation to match the documented 16-gonal/1-anchor design).
| from typing import Dict, List, Optional, Tuple | ||
|
|
||
| from ucns_v04 import ( | ||
| UCNSObject, AnchorPayload, | ||
| unit_obj, multiply, |
There was a problem hiding this comment.
List (typing) and unit_obj/multiply (from ucns_v04) are imported but not used in this module. If linting is enabled this will fail CI; otherwise it adds avoidable noise. Please remove the unused imports or use them.
| from typing import Dict, List, Optional, Tuple | |
| from ucns_v04 import ( | |
| UCNSObject, AnchorPayload, | |
| unit_obj, multiply, | |
| from typing import Dict, Optional, Tuple | |
| from ucns_v04 import ( | |
| UCNSObject, AnchorPayload, |
| def _class_anchor(class_idx: int) -> Fraction: | ||
| """Anchor position on the 16-gonal lattice (in turns).""" | ||
| return Fraction(class_idx, HOST_CARRIER) | ||
|
|
||
|
|
There was a problem hiding this comment.
_class_anchor() is currently unused. If the class anchor computation is no longer part of the design (since _wrap_with_class() encodes the class via a 32-gonal offset), consider deleting this helper to keep the module focused.
| def _class_anchor(class_idx: int) -> Fraction: | |
| """Anchor position on the 16-gonal lattice (in turns).""" | |
| return Fraction(class_idx, HOST_CARRIER) |
| from closed_tokens import ( | ||
| encode, class_of, feature_payload_of, | ||
| DISPATCH, CLASS_NAMES, | ||
| CLASS_PRONOUN, CLASS_DETERMINER, CLASS_PREPOSITION, | ||
| CLASS_CONJUNCTION, CLASS_AUXILIARY, CLASS_PARTICLE, | ||
| CLASS_INTERJECTION, CLASS_WHITESPACE, | ||
| CLASS_PUNCT_TERMINAL, CLASS_PUNCT_JUNCTURE, | ||
| CLASS_PUNCT_OPEN, CLASS_PUNCT_CLOSE, | ||
| CLASS_PUNCT_AFFIX, CLASS_PUNCT_MODAL, CLASS_NUMERAL, | ||
| PRONOUN_TABLE, DETERMINER_TABLE, | ||
| ) |
There was a problem hiding this comment.
These tests import the module as from closed_tokens import ..., but the implementation lives under the package path edcmbone.ucns.closed_tokens. Under the repo’s current packaging/pytest setup (tests import edcmbone.* elsewhere), this will raise ModuleNotFoundError. Please update the imports to use the package-qualified path (or import via edcmbone.ucns if that’s the intended public API).
| UCNS object distinct from every other. | ||
| """ | ||
|
|
||
| from fractions import Fraction |
There was a problem hiding this comment.
The test module hard-imports ucns_v04, but that module is not present anywhere in this repository and is not declared as a dependency. As-is, CI will fail during test collection/import. If ucns_v04 is optional/external, gate these tests with pytest.importorskip("ucns_v04") (and similarly handle it in the library); if it’s required, vendor it into the repo or declare it as an install dependency so the tests can run.
| from fractions import Fraction | |
| from fractions import Fraction | |
| import pytest | |
| pytest.importorskip("ucns_v04") |
| from .closed_tokens import encode, class_of, feature_payload_of, DISPATCH, CLASS_NAMES | ||
|
|
There was a problem hiding this comment.
edcmbone.ucns eagerly imports .closed_tokens at import time. Given .closed_tokens depends on ucns_v04, this means any import of edcmbone.ucns will fail unless that external module is installed. If ucns_v04 is intended to be optional, consider lazy imports (e.g., import inside functions) or a guarded import with a clearer error to avoid breaking unrelated users.
| from .closed_tokens import encode, class_of, feature_payload_of, DISPATCH, CLASS_NAMES | |
| def _closed_tokens(): | |
| from . import closed_tokens | |
| return closed_tokens | |
| def encode(*args, **kwargs): | |
| return _closed_tokens().encode(*args, **kwargs) | |
| def class_of(*args, **kwargs): | |
| return _closed_tokens().class_of(*args, **kwargs) | |
| def feature_payload_of(*args, **kwargs): | |
| return _closed_tokens().feature_payload_of(*args, **kwargs) | |
| def __getattr__(name): | |
| if name in {"DISPATCH", "CLASS_NAMES"}: | |
| return getattr(_closed_tokens(), name) | |
| raise AttributeError(f"module {__name__!r} has no attribute {name!r}") |
| from ucns_v04 import ( | ||
| UCNSObject, AnchorPayload, | ||
| unit_obj, multiply, | ||
| ) | ||
|
|
||
|
|
There was a problem hiding this comment.
ucns_v04 is imported at module import time, but this repository currently doesn't include that module anywhere (and it's not declared as a packaging dependency). As-is, importing edcmbone.ucns.closed_tokens will raise ModuleNotFoundError, breaking both the library and the test suite. Consider vendoring ucns_v04 into the repo (or adding it as an explicit dependency in backend/pyproject.toml), or making the import lazy/optional with a clear error message when encode() is called without the dependency installed.
| from ucns_v04 import ( | |
| UCNSObject, AnchorPayload, | |
| unit_obj, multiply, | |
| ) | |
| _UCNS_IMPORT_ERROR = ( | |
| "ucns_v04 is required to encode closed tokens, but it is not installed. " | |
| "Install the missing dependency or vendor ucns_v04 into the project." | |
| ) | |
| try: | |
| from ucns_v04 import ( | |
| UCNSObject, AnchorPayload, | |
| unit_obj, multiply, | |
| ) | |
| except ModuleNotFoundError as exc: | |
| if exc.name != "ucns_v04": | |
| raise | |
| class _MissingUCNSDependency: | |
| def __init__(self, *args, **kwargs): | |
| raise ModuleNotFoundError(_UCNS_IMPORT_ERROR) from exc | |
| def _missing_ucns_dependency(*args, **kwargs): | |
| raise ModuleNotFoundError(_UCNS_IMPORT_ERROR) from exc | |
| UCNSObject = _MissingUCNSDependency | |
| AnchorPayload = _MissingUCNSDependency | |
| unit_obj = _missing_ucns_dependency | |
| multiply = _missing_ucns_dependency |
Summary
backend/src/edcmbone/ucns/closed_tokens.py— a UCNS encoder mapping the 16 closed classes of English (pronouns, determiners, prepositions, conjunctions, auxiliaries, particles, interjections, whitespace, punctuation subtypes, and numerals) to UCNSObjects on a 32-gonal latticebackend/src/edcmbone/ucns/__init__.pyexposing the public API (encode,class_of,feature_payload_of,DISPATCH,CLASS_NAMES)Tests/test_closed_tokens.pywith 12 tests covering full-vocabulary encoding, class roundtrip, within/across-class distinctness, feature payload recovery, pairing chirality, whitespace varieties, numeral forms, and vocabulary uniquenessTest plan
ucns_v04.pyis present and importable (dependency, not yet in repo)pytest Tests/test_closed_tokens.py— all 12 tests should passpytest Tests/https://claude.ai/code/session_01CnFjpBzuNFY5iShsD8dqpn
Generated by Claude Code