Repository navigation
Refactor/semantic affinity routing - #21
Merged
Merged
Conversation
- Per-capability semantics grouping matching the engine's routing scope (cross-capability id reuse no longer mixes members, and seeded groups must stay single-capability) - Members must each recognize >=1 probe and every probe must be matched by >=1 member; dead members/probes fail loudly instead of passing via the previous bare 'continue' - Assert all matches agree with the expected notation, not just matches[0] - Drive each group's shared rule with its own contract class (Date/ Email/Phone) instead of a hardcoded DateContract - Correct the allowlist comment: coalesced ids vs renamed singletons
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughGrammar and rule affinity routing now uses semantic identifiers. Grammar metadata is validated at class definition time. The orchestrator validates and routes ChangesSemantic affinity routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Contract
participant Orchestrator
participant Grammar
participant Rule
Contract->>Orchestrator: Compose grammar names
Orchestrator->>Grammar: Resolve semantics
Grammar-->>Orchestrator: Return semantics identifiers
Orchestrator->>Rule: Validate target_semantics
Grammar->>Orchestrator: Emit recognition
Orchestrator->>Rule: Route matching semantic
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
paxman/core/domain.py (1)
205-243: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire each subclass to declare routing metadata.
hasattr()and the MRO fallback accept metadata inherited from a parent class. A subclass can then change recognition or validation behavior while retaining an unrelated inherited semantic target. The orchestrator will route that output incorrectly.Require required
Rulemetadata andGrammar.semanticsinvars(cls). Updatetests/unit/test_grammar_semantics_metadata.pyLines 108-125 to expect inheritedsemanticsto fail.Proposed fix
- missing = [attr for attr in required if not hasattr(cls, attr)] + missing = [attr for attr in required if attr not in vars(cls)] ... - value: Any = vars(cls).get(attribute, getattr(cls, attribute)) + value: Any = vars(cls)[attribute] ... - if not hasattr(cls, "semantics"): + if "semantics" not in vars(cls): raise TypeError(f"{cls.__name__} must define Grammar metadata: semantics") - semantics: Any = vars(cls).get("semantics", cls.semantics) + semantics: Any = vars(cls)["semantics"]As per coding guidelines:
Grammar subclasses must declare a non-empty string name and semantics;Rulesubclasses must declare valid class attributes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@paxman/core/domain.py` around lines 205 - 243, Require subclass-local metadata in Rule.__init_subclass__ and Grammar.__init_subclass__ by validating required attributes through vars(cls), not inherited MRO values; validate the declared Rule frozensets and non-empty target_semantics as before. Ensure Grammar subclasses declare a non-empty string name and semantics, and update test_grammar_semantics_metadata.py to expect inherited semantics to fail.Source: Coding guidelines
🧹 Nitpick comments (1)
tests/unit/test_grammar_semantics_consistency.py (1)
224-268: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a shipped grammar-to-rule coverage assertion.
Compare each capability’s
get_grammars()semantics with itsget_rules()target_semantics. Without this check, a grammar can recognize input while_collect_candidates()routes it to no rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_grammar_semantics_consistency.py` around lines 224 - 268, Add a unit assertion alongside test_every_shipped_grammar_belongs_to_one_semantics_group that compares each shipped capability’s get_grammars() semantics against the target_semantics values from get_rules(). Ensure every grammar semantics is covered by a rule target within the same capability, so _collect_candidates() cannot receive an unroutable grammar.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-08-11-adr0003-semantic-affinity-routing.md`:
- Around line 7-8: The plan and ADR contain conflicting precedence rules and
inventory counts. In
docs/superpowers/plans/2026-08-11-adr0003-semantic-affinity-routing.md:7-8,
retain an explicit authoritative-source rule; at :133-137, scope D10 to that
rule or remove the conflicting precedence statement; in
docs/adr/0003-semantic-affinity-routing.md:205-208, update the 55-file count to
the verified inventory or clearly label it as historical/non-authoritative.
In `@paxman/capabilities/Date/rules/en_50160_ed2010.py`:
- Line 33: Update target_semantics in the en_50160_ed2010 rule to remove
us_calendar_date, leaving only european_calendar_date so US dates are not
processed by the incompatible Section4DateFormat interpretation.
In `@paxman/capabilities/Date/rules/us_federal_rules_ed2023.py`:
- Line 33: Update the target_semantics declaration for Section1DateFormat to
include only the us_calendar_date semantic, removing european_calendar_date
unless a separate rule with the correct European field mapping is added.
In `@paxman/engine/orchestrator.py`:
- Around line 396-414: Update _activated_rules() so extra_grammars are resolved
only through known entries in semantics_by_name; ignore unknown grammar names
when computing extra_semantics instead of using the raw name as a fallback. Keep
dangling extension-target validation separate from activation, ensuring a
semantic identifier that is not a grammar name cannot activate community rules.
In `@tests/integration/test_grammar_extensions.py`:
- Line 152: Update the docstring for the dangling-target fixture to describe a
missing semantic identifier rather than a missing grammar, matching the test’s
unknown semantics assertion.
---
Outside diff comments:
In `@paxman/core/domain.py`:
- Around line 205-243: Require subclass-local metadata in Rule.__init_subclass__
and Grammar.__init_subclass__ by validating required attributes through
vars(cls), not inherited MRO values; validate the declared Rule frozensets and
non-empty target_semantics as before. Ensure Grammar subclasses declare a
non-empty string name and semantics, and update
test_grammar_semantics_metadata.py to expect inherited semantics to fail.
---
Nitpick comments:
In `@tests/unit/test_grammar_semantics_consistency.py`:
- Around line 224-268: Add a unit assertion alongside
test_every_shipped_grammar_belongs_to_one_semantics_group that compares each
shipped capability’s get_grammars() semantics against the target_semantics
values from get_rules(). Ensure every grammar semantics is covered by a rule
target within the same capability, so _collect_candidates() cannot receive an
unroutable grammar.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 53bd4ea5-6a09-49d8-b147-a7dbb6cb6fad
📒 Files selected for processing (76)
ARCHITECTURE.mdCONTEXT.mdHOW_TO_ADD_NEW_CAPABILITY.mdHOW_TO_ADD_NEW_GRAMMAR.mdREADME.mdcapability_homogeneity_audit.mddocs/adr/0003-semantic-affinity-routing.mddocs/superpowers/plans/2026-08-11-adr0003-semantic-affinity-routing.mdpaxman/capabilities/AGENTS.mdpaxman/capabilities/Country/grammar/alpha2_recognition.pypaxman/capabilities/Country/grammar/alpha3_recognition.pypaxman/capabilities/Country/grammar/name_recognition.pypaxman/capabilities/Country/grammar/numeric_recognition.pypaxman/capabilities/Country/rules/cldr_localized_ed2025.pypaxman/capabilities/Country/rules/iso_3166_ed2024.pypaxman/capabilities/Country/rules/iso_3166_historical_ed2020.pypaxman/capabilities/Currency/grammar/code_recognition.pypaxman/capabilities/Currency/grammar/symbol_recognition.pypaxman/capabilities/Currency/grammar/word_recognition.pypaxman/capabilities/Currency/rules/cldr_currencies_ed2025.pypaxman/capabilities/Currency/rules/iso_4217_ed2015.pypaxman/capabilities/Date/grammar/european_recognition.pypaxman/capabilities/Date/grammar/iso8601_recognition.pypaxman/capabilities/Date/grammar/slash_iso_recognition.pypaxman/capabilities/Date/grammar/us_recognition.pypaxman/capabilities/Date/rules/en_50160_ed2010.pypaxman/capabilities/Date/rules/iso_8601_ed2019.pypaxman/capabilities/Date/rules/us_federal_rules_ed2023.pypaxman/capabilities/Email/grammar/localhost_recognition.pypaxman/capabilities/Email/grammar/obfuscated_recognition.pypaxman/capabilities/Email/grammar/standard_recognition.pypaxman/capabilities/Email/rules/rfc_5322_ed2008.pypaxman/capabilities/Email/rules/rfc_6761_ed2012.pypaxman/capabilities/IP/grammar/ipv4_recognition.pypaxman/capabilities/IP/grammar/ipv6_recognition.pypaxman/capabilities/IP/rules/rfc_5952_ed2010.pypaxman/capabilities/IP/rules/rfc_791_ed1981.pypaxman/capabilities/ISBN/grammar/isbn10_recognition.pypaxman/capabilities/ISBN/grammar/isbn13_recognition.pypaxman/capabilities/ISBN/rules/isbn_range_message_ed2026.pypaxman/capabilities/ISBN/rules/isbn_users_manual_ed2012.pypaxman/capabilities/ISBN/rules/iso_2108_ed2017.pypaxman/capabilities/Money/grammar/code_recognition.pypaxman/capabilities/Money/grammar/symbol_recognition.pypaxman/capabilities/Money/grammar/word_recognition.pypaxman/capabilities/Money/rules/cldr_currencies_ed2025.pypaxman/capabilities/Money/rules/iso_4217_ed2015.pypaxman/capabilities/Phone/grammar/e164_recognition.pypaxman/capabilities/Phone/grammar/international_00_recognition.pypaxman/capabilities/Phone/grammar/national_recognition.pypaxman/capabilities/Phone/grammar/tel_uri_recognition.pypaxman/capabilities/Phone/rules/e164_ed2010.pypaxman/capabilities/Phone/rules/nanp_ed2024.pypaxman/capabilities/Phone/rules/rfc_3966_ed2004.pypaxman/capabilities/URL/grammar/absolute_uri_recognition.pypaxman/capabilities/URL/rules/whatwg_url_standard.pypaxman/core/AGENTS.mdpaxman/core/domain.pypaxman/core/extensions.pypaxman/engine/orchestrator.pytests/capabilities/currency/test_rules.pytests/capabilities/date/test_grammar.pytests/capabilities/isbn/test_rules.pytests/capabilities/money/test_rules.pytests/capabilities/url/test_rule.pytests/integration/test_feature_gating.pytests/integration/test_format_value_seam.pytests/integration/test_grammar_extensions.pytests/integration/test_pipeline.pytests/integration/test_recognition_seam.pytests/unit/test_capability.pytests/unit/test_discovery.pytests/unit/test_extensions.pytests/unit/test_grammar_semantics_consistency.pytests/unit/test_grammar_semantics_metadata.pytests/unit/test_rule_metadata.py
| provenance = PUBLICATION | ||
| citation = "Section 4 (date format)" | ||
| target_grammars = frozenset({"us_recognition", "european_recognition"}) | ||
| target_semantics = frozenset({"us_calendar_date", "european_calendar_date"}) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'class .*Grammar|semantics\s*=|DateNotation\(' \
paxman/capabilities/Date/grammar
rg -n -C 8 \
'us_calendar_date|european_calendar_date|Section4DateFormat|target_semantics' \
paxman/capabilities/Date/rules \
tests/unit/test_grammar_semantics_consistency.pyRepository: azaharizaman/paxman-python
Length of output: 25729
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '22,130p' paxman/capabilities/Date/rules/en_50160_ed2010.py
sed -n '1,120p' tests/unit/test_grammar_semantics_consistency.py
python3 - <<'PY'
import ast
from pathlib import Path
rule_path = Path("paxman/capabilities/Date/rules/en_50160_ed2010.py")
us_path = Path("paxman/capabilities/Date/grammar/us_recognition.py")
eu_path = Path("paxman/capabilities/Date/grammar/european_recognition.py")
def class_assignments(path, class_name):
tree = ast.parse(path.read_text())
cls = next(n for n in tree.body if isinstance(n, ast.ClassDef) and n.name == class_name)
values = {}
for node in cls.body:
if isinstance(node, ast.Assign) and len(node.targets) == 1:
target = node.targets[0]
if isinstance(target, ast.Name):
values[target.id] = ast.literal_eval(node.value) if node.id if False else None
return cls
def mapping(path):
text = path.read_text()
tree = ast.parse(text)
cls = next(n for n in tree.body if isinstance(n, ast.ClassDef))
result = {}
for node in cls.body:
if isinstance(node, ast.Assign) and len(node.targets) == 1:
name = node.targets[0]
if isinstance(name, ast.Name) and name.id == "semantics":
result["semantics"] = ast.literal_eval(node.value)
for node in ast.walk(cls):
if isinstance(node, ast.Assign) and len(node.targets) == 1:
name = node.targets[0]
if isinstance(name, ast.Name) and name.id == "notation":
pass
return result
print("grammar semantics:", mapping(us_path), mapping(eu_path))
print("US notation groups for 03/04/2026: N1=03 (month), N2=04 (day), N3=2026")
print("EN rule documentation: N1=day, N2=month, N3=year")
print("Therefore the same notation would be interpreted as 3 April by US grammar but 4 March by EN rule.")
PYRepository: azaharizaman/paxman-python
Length of output: 7110
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '22,130p' paxman/capabilities/Date/rules/en_50160_ed2010.py
sed -n '120,310p' tests/unit/test_grammar_semantics_consistency.py
python3 - <<'PY'
import ast
from pathlib import Path
def grammar_semantics(path):
tree = ast.parse(Path(path).read_text())
cls = next(node for node in tree.body if isinstance(node, ast.ClassDef))
for node in cls.body:
if (
isinstance(node, ast.Assign)
and len(node.targets) == 1
and isinstance(node.targets[0], ast.Name)
and node.targets[0].id == "semantics"
):
return ast.literal_eval(node.value)
raise AssertionError(f"semantics not found in {path}")
print("us_recognition semantics:", grammar_semantics(
"paxman/capabilities/Date/grammar/us_recognition.py"
))
print("european_recognition semantics:", grammar_semantics(
"paxman/capabilities/Date/grammar/european_recognition.py"
))
print("US grammar notation for 03/04/2026: N1=03 month, N2=04 day, N3=2026 year")
print("EN 50160 rule notation: N1=day, N2=month, N3=year")
print("The same notation would resolve to 3 April under US semantics and 4 March under the EN 50160 rule.")
PYRepository: azaharizaman/paxman-python
Length of output: 9583
Remove us_calendar_date from target_semantics.
us_recognition emits N1=month and N2=day, but Section4DateFormat interprets them as day and month. This swaps or rejects US dates during canonicalization. Keep this rule scoped to european_calendar_date, or add a US-specific rule.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@paxman/capabilities/Date/rules/en_50160_ed2010.py` at line 33, Update
target_semantics in the en_50160_ed2010 rule to remove us_calendar_date, leaving
only european_calendar_date so US dates are not processed by the incompatible
Section4DateFormat interpretation.
| provenance = PUBLICATION | ||
| citation = "Section 1 (date format)" | ||
| target_grammars = frozenset({"us_recognition", "european_recognition"}) | ||
| target_semantics = frozenset({"us_calendar_date", "european_calendar_date"}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restrict Section1DateFormat to the US calendar semantic.
Line 33 also targets european_calendar_date, but this rule interprets N1 as month and N2 as day. That mapping is valid for the documented US format, not for DD/MM/YYYY European notation. Restrict the target set to {"us_calendar_date"}, or add a separate rule with the European field mapping.
Proposed metadata fix
- target_semantics = frozenset({"us_calendar_date", "european_calendar_date"})
+ target_semantics = frozenset({"us_calendar_date"})The supplied Section1DateFormat notation mapping establishes the US-only field order.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| target_semantics = frozenset({"us_calendar_date", "european_calendar_date"}) | |
| target_semantics = frozenset({"us_calendar_date"}) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@paxman/capabilities/Date/rules/us_federal_rules_ed2023.py` at line 33, Update
the target_semantics declaration for Section1DateFormat to include only the
us_calendar_date semantic, removing european_calendar_date unless a separate
rule with the correct European field mapping is added.
Comment on lines
+396
to
+414
| capability: Capability[Any], | ||
| contract: Contract, | ||
| semantics_by_name: dict[str, str], | ||
| ) -> list[Rule[Any]]: | ||
| """Community rules opt in like grammars: a rule runs only when the | ||
| contract names one of its ``target_grammars`` in ``extra_grammars``. | ||
| contract's ``extra_grammars`` resolve to one of its ``target_semantics``. | ||
|
|
||
| An un-opted community rule — even one targeting a shipped grammar — | ||
| never affects results, keeping extension behavior deterministic per | ||
| contract. | ||
| An unknown extra name keeps its own string as the semantics key, so a | ||
| rule declaring it (a dangling target) still fails fast in affinity | ||
| validation instead of being silently excluded. An un-opted community | ||
| rule — even one targeting a shipped grammar — never affects results, | ||
| keeping extension behavior deterministic per contract. | ||
| """ | ||
| extra_grammars = set(getattr(contract, "extra_grammars", ())) | ||
| extra_semantics = {semantics_by_name.get(n, n) for n in extra_grammars} | ||
| return [ | ||
| rule | ||
| for rule in get_extended_rules(capability.name) | ||
| if extra_grammars & rule.target_grammars | ||
| if extra_semantics & rule.target_semantics |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not convert an unknown grammar name into a semantic identifier.
At Line 410, an unknown extra_grammars entry becomes its own semantic identifier. If that string equals a shipped semantic identifier but not a grammar name, _activated_rules() activates a community rule while _recognize() skips the requested grammar. The result can change although no community grammar was opted in.
Resolve only known grammar names here. If dangling extension targets need validation, validate them separately.
Proposed fix
- extra_semantics = {semantics_by_name.get(n, n) for n in extra_grammars}
+ extra_semantics = {
+ semantics_by_name[name]
+ for name in extra_grammars
+ if name in semantics_by_name
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| capability: Capability[Any], | |
| contract: Contract, | |
| semantics_by_name: dict[str, str], | |
| ) -> list[Rule[Any]]: | |
| """Community rules opt in like grammars: a rule runs only when the | |
| contract names one of its ``target_grammars`` in ``extra_grammars``. | |
| contract's ``extra_grammars`` resolve to one of its ``target_semantics``. | |
| An un-opted community rule — even one targeting a shipped grammar — | |
| never affects results, keeping extension behavior deterministic per | |
| contract. | |
| An unknown extra name keeps its own string as the semantics key, so a | |
| rule declaring it (a dangling target) still fails fast in affinity | |
| validation instead of being silently excluded. An un-opted community | |
| rule — even one targeting a shipped grammar — never affects results, | |
| keeping extension behavior deterministic per contract. | |
| """ | |
| extra_grammars = set(getattr(contract, "extra_grammars", ())) | |
| extra_semantics = {semantics_by_name.get(n, n) for n in extra_grammars} | |
| return [ | |
| rule | |
| for rule in get_extended_rules(capability.name) | |
| if extra_grammars & rule.target_grammars | |
| if extra_semantics & rule.target_semantics | |
| capability: Capability[Any], | |
| contract: Contract, | |
| semantics_by_name: dict[str, str], | |
| ) -> list[Rule[Any]]: | |
| """Community rules opt in like grammars: a rule runs only when the | |
| contract's ``extra_grammars`` resolve to one of its ``target_semantics``. | |
| An unknown extra name keeps its own string as the semantics key, so a | |
| rule declaring it (a dangling target) still fails fast in affinity | |
| validation instead of being silently excluded. An un-opted community | |
| rule — even one targeting a shipped grammar — never affects results, | |
| keeping extension behavior deterministic per contract. | |
| """ | |
| extra_grammars = set(getattr(contract, "extra_grammars", ())) | |
| extra_semantics = { | |
| semantics_by_name[name] | |
| for name in extra_grammars | |
| if name in semantics_by_name | |
| } | |
| return [ | |
| rule | |
| for rule in get_extended_rules(capability.name) | |
| if extra_semantics & rule.target_semantics |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@paxman/engine/orchestrator.py` around lines 396 - 414, Update
_activated_rules() so extra_grammars are resolved only through known entries in
semantics_by_name; ignore unknown grammar names when computing extra_semantics
instead of using the raw name as a fallback. Keep dangling extension-target
validation separate from activation, ensuring a semantic identifier that is not
a grammar name cannot activate community rules.
Oracle review follow-up (ADR-0003):
- test_grammar_semantics_metadata: assert us_recognition -> us_calendar_date
and european_recognition -> european_calendar_date, closing the cross-swap
hole in the identity allowlist (D7 guard now pins name->id mapping)
- test_grammar_extensions: extra_grammars=("iso8601_calendar_date",) activates
a rule targeting that semantics id without opting in any community grammar,
locking the README-documented semantics_by_name.get(n, n) fallback
Oracle review follow-up (ADR-0003): - ADR + plan D10: verified inventory is 55 files (44 sweep-relevant: 26 under paxman/, 12 tests, 6 repo-root docs incl HOW_TO_ADD_NEW_GRAMMAR.md) - ADR Phase 1 + plan out-of-scope: post-ADR addendum that the date grammars' digit-lookaround bounds deliberately stop digit-glued ids like 12026-01-15 from partially matching - plan: sync progress table (all ten tasks landed)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/superpowers/plans/2026-08-11-adr0003-semantic-affinity-routing.md (2)
73-73: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winQualify the byte-identical behavior claim.
The sentence beginning at Line 73 claims byte-identical output for every existing input. Lines 161-164 document an intentional behavior change for digit-glued date inputs. Update the claim to exclude that input class.
Also applies to: 161-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-08-11-adr0003-semantic-affinity-routing.md` at line 73, Update the byte-identical output claim in the ADR around the provenance and candidate-dedup discussion to explicitly exclude digit-glued date inputs, and revise the corresponding statement at the documented behavior-change section around the digit-glued date examples. Preserve the claim for all other existing inputs.
652-660: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the zero-grep proof include every required path.
Lines 652-660 exclude
paxman/andtests/, but Task 9 includespaxman/core/AGENTS.mdandpaxman/capabilities/AGENTS.md. Lines 750-751 also require zero hits inpaxman/andtests/. Remove those exclusions. Do not convert grep errors intoCLEANwith|| echo CLEAN.Proposed command change
- --exclude-dir=paxman --exclude-dir=tests \ + # Keep paxman/ and tests/ in scope.Also applies to: 750-751
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-08-11-adr0003-semantic-affinity-routing.md` around lines 652 - 660, Update the zero-grep proof command in the documented sweep, including the corresponding command at the later proof section, to search paxman/ and tests/ by removing those --exclude-dir options. Remove the || echo "CLEAN" fallback so grep errors remain visible instead of being reported as a clean result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-08-11-adr0003-semantic-affinity-routing.md`:
- Line 35: Align the Task 10 status with the documented validation scope: either
mark the gate green only for CI-scoped formatting, or replace the full-tree ruff
format command with the CI-scoped command. Update the related statements at the
Task 10 summary and the validation sections so the status and command
consistently describe the same scope.
---
Outside diff comments:
In `@docs/superpowers/plans/2026-08-11-adr0003-semantic-affinity-routing.md`:
- Line 73: Update the byte-identical output claim in the ADR around the
provenance and candidate-dedup discussion to explicitly exclude digit-glued date
inputs, and revise the corresponding statement at the documented behavior-change
section around the digit-glued date examples. Preserve the claim for all other
existing inputs.
- Around line 652-660: Update the zero-grep proof command in the documented
sweep, including the corresponding command at the later proof section, to search
paxman/ and tests/ by removing those --exclude-dir options. Remove the || echo
"CLEAN" fallback so grep errors remain visible instead of being reported as a
clean result.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e1ea1e02-5e10-4b65-a38c-a7bf8657add2
📒 Files selected for processing (5)
docs/adr/0003-semantic-affinity-routing.mddocs/superpowers/plans/2026-08-11-adr0003-semantic-affinity-routing.mdtests/integration/test_grammar_extensions.pytests/unit/test_grammar_semantics_consistency.pytests/unit/test_grammar_semantics_metadata.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/unit/test_grammar_semantics_metadata.py
- docs/adr/0003-semantic-affinity-routing.md
Review follow-up (ADR-0003): - Task 10 gate command now matches CI: ruff check/format scoped to paxman/ and tests/ (the full-tree command never was CI-authoritative) - ADR section 4 + plan goal: byte-identical output claim explicitly excludes digit-glued dates (post-ADR lookaround tightening) - Sweep proofs: Task 9 command now searches paxman/ and tests/ (the nested AGENTS.md are swept there); drop the || echo "CLEAN" fallbacks in both commands so grep errors (exit >= 2) stay visible instead of being reported as a clean result; Task 10 and DoD statements updated to match
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
1 participant
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.
Summary by CodeRabbit