Rectify: Architectural Constraint Catalog Discovery Immunity#3370
Merged
Trecek merged 4 commits intoMay 31, 2026
Merged
Conversation
Trecek
force-pushed
the
resolve-review-architectural-constraint-catalog-missing-ment/3331
branch
from
ddae1c0 to
7edb636
Compare
Trecek
commented
May 31, 2026
Trecek
commented
May 31, 2026
| f"Catalog references {ref} but it does not exist anywhere in tests/" | ||
| ) | ||
|
|
||
|
|
Collaborator
Author
There was a problem hiding this comment.
[warning] defense: catalog_section is extracted with a hardcoded 6000-character slice (skill_text[catalog_idx : catalog_idx + 6000]). The catalog section has grown significantly (20+ new rows added in this PR). If it exceeds 6000 chars, test filenames beyond that boundary will be silently missed, producing false-negative passes. The same pattern appears in test_catalog_forward_references_valid and test_catalog_completeness_via_discovery — all three occurrences need the same fix: use a section-delimiter approach instead of a magic constant.
Collaborator
Author
There was a problem hiding this comment.
Acknowledged — minor suggestion noted. Already addressed in 7edb636 (section-delimiter extraction replaces hardcoded 6000-char slice).
Trecek
commented
May 31, 2026
Trecek
commented
May 31, 2026
Trecek
commented
May 31, 2026
Trecek
commented
May 31, 2026
Collaborator
Author
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review: blocking issues found. See inline comments.
⚠️ Outside Diff Range
These findings target lines not in the diff and could not be posted as inline comments:
tests/skills/test_resolve_review_arch_constraint_awareness.py
- L176 [warning/defense]:
catalog_sectionis extracted with a hardcoded 6000-character slice intest_catalog_forward_references_valid. Same fix needed as L195 — use section-delimiter extraction instead of a magic constant.
…onvention discovery
Adds `tests/_arch_constraint_discovery.py` which scans the full `tests/`
tree for test files whose module docstring starts with a recognized prefix
(Structural guard, AST guard, Architectural invariant, etc.).
Rewrites the two hardcoded staleness guards in
`test_resolve_review_arch_constraint_awareness.py`:
- `test_catalog_reverse_coverage`: now calls `discover_constraint_tests()`
and asserts all non-excluded constraint tests appear in the SKILL.md catalog
- `test_catalog_forward_references_valid`: now uses `rglob("test_*.py")`
from the full tests/ tree instead of hard-coded `tests/arch/` + `tests/recipe/`
Adds two new tests:
- `test_discovery_finds_known_constraints`: self-test confirming cross-directory
discovery works (arch/, workspace/, recipe/)
- `test_catalog_completeness_via_discovery`: bidirectional completeness guard
Normalizes four non-conforming constraint test docstrings to the recognized
prefix convention so discovery finds them:
- `test_skills.py`: "Tests for..." → "Structural guard: ..."
- `test_clone_timeouts.py`: "Static analysis: ..." → "Structural guard: ..."
- `test_no_path_cwd_in_tools.py`: "Regression guard: ..." → "Structural guard: ..."
- `test_anti_pattern_guards.py`: "Guards against..." → "Structural guard: ..."
Expands the SKILL.md Architectural Constraint Catalog from 13 rows (covering
only tests/arch/ and tests/recipe/) to 38 rows covering all 31 discoverable
constraint tests that can be violated by reviewer suggestions. Adds the
originally-missing @-mention guard entry (`test_skills.py`) plus 24 others.
Updates supporting artifacts: `tests/_test_filter.py` BUCKET_A_PATTERNS,
`tests/CLAUDE.md` helper listing, `tests/skills/CLAUDE.md` description.
…g in discover_constraint_tests
…elimiter extraction
Trecek
force-pushed
the
resolve-review-architectural-constraint-catalog-missing-ment/3331
branch
from
7edb636 to
c29a46d
Compare
Trecek
deleted the
resolve-review-architectural-constraint-catalog-missing-ment/3331
branch
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
The resolve-review skill's Architectural Constraint Catalog is a manually curated table in SKILL.md that lists pytest-enforced constraints. Its staleness guard (
test_catalog_reverse_coverage) uses a hardcoded file set and only searchestests/arch/andtests/recipe/. This means new constraint tests added to any other directory (e.g.,tests/workspace/,tests/server/,tests/execution/) silently go uncatalogued. The @-mention guard (test_no_hardcoded_username_mentions_in_skill_mdsintests/workspace/test_skills.py) is the immediate victim, but the pattern failure is broader: 52 structural/architectural guard tests exist across 7 directories (expanding to 56 tests across 9 directories after docstring normalization), the catalog lists only 13, and the staleness guard can only ever see tests in 2 of those directories.The architectural solution: replace the manually curated catalog and its manually curated staleness guard with docstring-convention-based automatic discovery. The codebase already has a strong convention — most constraint tests use one of a small set of docstring prefixes. A discovery test can find all constraint tests by convention, validate the catalog covers them, and fail immediately when a new constraint test is added anywhere without a catalog entry.
Closes #3331
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/remediation-20260530-174543-571962/.autoskillit/temp/rectify/rectify_arch_constraint_catalog_discovery_2026-05-30_175800.md🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary
* Step used a non-Anthropic provider; caching behavior may differ.
Token Efficiency
Model Usage Breakdown