Fix #49: block false-positive affix bones with canon stem validation - #65
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens affix detection by validating stripped residuals against canon stems, while also adjusting apostrophe normalization and test organization.
Changes:
- Adds canon-backed residual validation for backend parser affix emission and optional stem gating in the core matcher.
- Normalizes smart apostrophes to ASCII and adds regression tests for contractions and affix false positives.
- Moves pytest collection toward lowercase
tests/and removes older uppercase/Backend artifacts.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
backend/src/edcmbone/parser/turns_rounds.py |
Updates tokenizer and affix residual validation in the backend parser. |
core/operator/matcher.py |
Adds optional valid_stems gating to affix matching. |
core/parsing/normalizer.py |
Normalizes smart apostrophe variants to ASCII apostrophes. |
tests/test_affix_residual_validation.py |
Adds regression coverage for affix residual validation. |
tests/test_apostrophe_normalization_and_tokenization.py |
Adds regression coverage for apostrophe normalization/tokenization. |
backend/src/edcmbone/ucns/ucns_v04.py |
Adds packaged UCNS implementation under backend source tree. |
backend/src/edcmbone/ucns/closed_tokens.py |
Updates UCNS import to package-relative form. |
backend/pyproject.toml |
Changes pytest collection path to lowercase tests. |
Tests/test_backend.py |
Removes old uppercase backend regression suite. |
Tests/test_closed_tokens.py |
Removes old uppercase closed-token test copy. |
Documentation/spec.md |
Removes old documentation file. |
Backend/requirements.txt |
Removes old uppercase Backend requirements file. |
Backend/LICENSE |
Removes old uppercase Backend license copy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Split into word-runs and individual punctuation characters | ||
| _WORD_RE = re.compile(r"[A-Za-z''\-]+|[^\w\s]|\d+") | ||
| _WORD_RE = re.compile(r"[A-Za-z]+(?:'[A-Za-z]+)*|[0-9]+|[^\w\s]") |
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Implemented in commit d0f0119. The backend parser tokenizer now preserves hyphenated compounds as a single surface token by matching [A-Za-z0-9]+(?:-[A-Za-z0-9]+)+ before punctuation, and I added a regression test asserting state-of-the-art stays one token.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot apply changes based on this feedback
| residual = root[len(p):] | ||
| if valid_stems is not None and residual not in valid_stems: | ||
| continue |
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Fixed in commit 9738a77. The match_affixes function now defers validation until after all affixes are stripped, allowing multi-affix words like "redoing" (re+do+ing) to work correctly even when intermediate forms like "doing" are not in valid_stems.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot apply changes based on this feedback
| residual = root[:-len(s)] | ||
| if valid_stems is not None and residual not in valid_stems: | ||
| continue |
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Fixed in commit 9738a77. Both prefix and suffix validation now happen after all affixes are stripped, so words like "redoing" can emit both "re" and "ing" affixes with final root "do" validated against valid_stems.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot apply changes based on this feedback
![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: a893814fd9
ℹ️ 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".
|
|
||
| # Split into word-runs and individual punctuation characters | ||
| _WORD_RE = re.compile(r"[A-Za-z''\-]+|[^\w\s]|\d+") | ||
| _WORD_RE = re.compile(r"[A-Za-z]+(?:'[A-Za-z]+)*|[0-9]+|[^\w\s]") |
There was a problem hiding this comment.
Restore hyphen handling in this regex: removing - from the word branch now splits compounds like state-of-the-art into ['state', '-', 'of', '-', 'the', '-', 'art'], so the classifier emits a punctuation bone for each hyphen and inflates K counts. This contradicts the canon rule that hyphenated compounds emit one K per compound word, and it changes structural metrics for any hyphen-rich input.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- I inspected
backend/src/edcmbone/parser/turns_rounds.pyat the requested path and confirmed the hyphenated-compound tokenizer repair is already present:_WORD_REmatches compound forms like[A-Za-z0-9]+(?:-[A-Za-z0-9]+)+before punctuation, which preserves tokens likestate-of-the-artas a single surface token. backend/src/edcmbone/parser/turns_rounds.pyL207-L208 - I did not make any additional code changes because the requested repair appears to already be applied in the current branch/worktree (clean
git status).
Testing
- ✅
cd /workspace/edcmbone && pytest -q(all tests passing:21 passed).
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>
|
@copilot resolve the merge conflicts in this pull request |
Agent-Logs-Url: https://github.com/The-Interdependency/edcmbone/sessions/73154e0c-bf87-4066-b51c-d52b607c3e6f Co-authored-by: erinepshovel-code <250928284+erinepshovel-code@users.noreply.github.com>
…pped Agent-Logs-Url: https://github.com/The-Interdependency/edcmbone/sessions/13182a36-ab97-4a20-91ff-5e43ffad6f5e Co-authored-by: erinepshovel-code <250928284+erinepshovel-code@users.noreply.github.com>
Resolved conflicts by keeping our improved implementations: - Tokenizer with hyphen compound support - Deferred affix validation in core/operator/matcher.py - Backend parser with _valid_stems set approach All tests pass (21 passed). Co-authored-by: erinepshovel-code <250928284+erinepshovel-code@users.noreply.github.com>
Motivation
unit -> un- + it).Description
self._valid_stemsset in the backend parser_BoneClassifierfromCanonLoader.all_words()and exclude entries withprimary == "S"to avoid function-word residuals.backend/src/edcmbone/parser/turns_rounds.py::classify_sequence, skip emitting prefix/suffix affix bones unless the strippedresidualis found inself._valid_stems(applies to both prefix and suffix paths).valid_stemsparameter tocore/operator/matcher.match_affixesand use it to gate prefix/suffix matches so the core matcher can perform equivalent residual validation when desired.tests/:tests/test_affix_residual_validation.pywhich asserts negative cases (uncle,unit,universe,under,unique) and positive/canon-dependent cases (e.g.,redo,unhappy/linking).Testing
pytest tests/test_affix_residual_validation.pyand fixed a failing assertion by excludingprimary == "S"entries from the stem index; final run:4 passed in 0.04s.pytest tests/test_affix_residual_validation.py(initial run showed 1 failure), update applied, thenpytest tests/test_affix_residual_validation.py(final run:4 passed).Codex Task