Repository navigation
feat(repo): repo budget counts .claude/rules - #326
Conversation
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…d rules dir, claude_rules key Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…irst Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| > `.claude/rules/**/*.md` files are `rule` entries: at start in the root `.claude/rules/` unless | ||
| > their frontmatter's `paths` names a pattern, lazily then and in a subdirectory's `.claude/rules/`; | ||
| > an `@` path in a rule is not followed. The `start` total is not an upper bound: it is too low by |
There was a problem hiding this comment.
Must-fix: these rules omit the case where guidance imports a rule. For example, with CLAUDE.md containing @.claude/rules/api.md and api.md containing paths: src/** frontmatter, repo budget reports the rule as start, not lazy (also asserted by test_a_rule_a_root_claude_md_imports_is_one_start_entry). If that rule contains @extra.md, _imports also follows it transitively. Qualify these statements to distinguish automatic rule discovery from guidance imports, or adjust the implementation if the unconditional no-import behavior is intended. The same missing start-classification exception appears in docs/architecture.md and the changelog; the changelog also repeats the unconditional no-import claim.
There was a problem hiding this comment.
Waiting on a decision before I change this: is the harness's @ expansion inside a rule that a root CLAUDE.md imports part of the behaviour we document? Context: the code follows those @ paths transitively, as it does for any imported file, and counts them as start imports; the docs say an @ path in a rule is not followed, without the imported-rule case. Options: (a) keep the current behaviour and state it in the docs, with the start classification of imported rules; (b) stop expansion at an imported rule. My recommendation: measure it on Claude Code with repo reach before choosing; I can make the doc change in the next round once it is answered. The thread stays open until then.
There was a problem hiding this comment.
On the question in this thread: I would keep the current code (option a) and fix the docs. Claude Code's memory docs say an imported file can import more files, up to four hops; a file a CLAUDE.md @-imports is loaded as an import, whatever directory it is in. What 2.1.304 measured is a rule loaded as a rule (@extra.md not expanded), and that does not cover a rule a CLAUDE.md imports. With the PR code, CLAUDE.md = @.claude/rules/api.md, api.md with paths: src/** and see @../../extra.md gives api.md start and extra.md a start import, which matches the docs. So the docs should say: "an @ path in a rule is not followed, unless a CLAUDE.md imports the rule: then the rule loads at start and its imports are followed" (here, in architecture.md and in the changelog fragment). One thing this does not settle: when imported, the rule's frontmatter is probably not removed, but it is not counted now. repo reach on a probe repo would measure both (a few haiku calls, cents).
There was a problem hiding this comment.
Waiting on a decision before I change this, still open. The reviewer's comment is one reading (keep following @ paths, and state it), not the author's decision.
Options: (a) keep following them, transitively and hop-limited, and state that in these docs, architecture.md and the changelog, as the comment proposes; (b) stop expansion at an imported rule.
My recommendation: (a), with the wording from the comment, after repo reach measures the imported-rule case on Claude Code. The comment also raises a new question for that measurement: whether an imported rule keeps its frontmatter. The count strips it on this route, as on the rule route, and that is not measured yet. The thread stays open until then.
There was a problem hiding this comment.
Done in f4b58f8: maintainer decision (a). An @ path in a rule that a CLAUDE.md imports is followed, transitively and within import_max_depth (4 for claude-code). Pinned by test_a_rule_a_claude_md_imports_expands_its_at_imports_transitively and test_an_import_chain_through_a_rule_counts_toward_the_hop_limit.
| unreadable_warnings.append(warning) | ||
| entries.append({"path": f, "kind": "guidance", "when": when, "chars": chars}) | ||
| counted = {_target(tree, f) for f in guidance} | ||
| entries += _rules(tree, files if rules else [], counted, start_t, unreadable_warnings) |
There was a problem hiding this comment.
Must-fix: rule discovery happens after _imports has already traversed the rule's body, so the promised non-expansion of a rule's @ paths is not enforced for imported rules. For example, CLAUDE.md containing @.claude/rules/api.md, .claude/rules/api.md containing @../../extra.md, and extra.md containing extra\n produces a startup import entry for extra.md. _rules only deduplicates the rule itself; it cannot remove that transitive import or its hop-limit warnings. Make import traversal aware of rules when this profile feature is enabled, stopping expansion at a rule while still counting the explicitly imported rule, and add a regression test for this chain.
There was a problem hiding this comment.
Waiting on a decision before I change this: the same question as in the docs thread, since this one asks for the non-expansion to be enforced. Should a @ path in a rule that a root CLAUDE.md imports be followed (current code, transitively, hop-limited) or not (your proposal)? My recommendation: measure it on Claude Code first; I will make the change and add the regression test the same round once you pick. The thread stays open until then.
There was a problem hiding this comment.
Done in f4b58f8: docs/repo-evaluation-contract.md, docs/architecture.md and the changelog fragment now say the same: an @ path in a rule is not followed unless a CLAUDE.md imports the rule, then its @ imports are followed within the hop limit. Frontmatter is still not scanned (test_an_at_path_in_a_rules_frontmatter_is_not_an_import).
| if _target(tree, f) in counted: | ||
| continue # a link to a file already counted, or a rule that is also a guidance file |
There was a problem hiding this comment.
Must-fix: deduplicating against guidance before considering the rule's loading time loses startup context. For example, create CLAUDE.md with no imports, AGENTS.md containing agents\n, and .claude/rules/agents.md symlinked to ../../AGENTS.md. The guidance entry is classified off, then this branch skips the unconditional root rule, leaving its 7 characters out of start and incorrectly warning that none of AGENTS.md is read. Similarly, a root rule linked to sub/CLAUDE.md leaves that content classified lazy. Merge the loading conditions of rule and guidance aliases before deduplicating (promoting the retained entry to start when appropriate), and add regression tests for both cases.
There was a problem hiding this comment.
Fixed in 21e287c: a rule linked to a guidance file is the same bytes, so the guidance entry now takes the earlier of the two loading conditions instead of the rule being dropped. The AGENTS.md a root rule links to is start, and its unread warning is gone; a rule linked to sub/CLAUDE.md counts that file at start. Pinned by test_a_root_rule_linked_to_an_off_agents_md_loads_it_at_start and test_a_root_rule_linked_to_a_nested_guidance_file_loads_it_at_start.
| """True if ``paths`` names at least one pattern: a list item or an inline value not ``[]`` or ``""``.""" | ||
| try: | ||
| meta = yaml.safe_load(frontmatter) | ||
| except (yaml.YAMLError, RecursionError): # RecursionError: nesting too deep for the parser |
There was a problem hiding this comment.
Must-fix: yaml.safe_load can also raise ValueError when constructing an oversized decimal integer. On supported Python 3.11+ with the default conversion limit, a rule whose frontmatter is "unused: " + "9" * 5000 + "\n" crashes repo budget, even though it has no paths field. Neither _rule nor the command catches this parser exception. Include ValueError in the fallback handling and add a regression test with an oversized scalar unrelated to paths.
Suggestion:
| except (yaml.YAMLError, RecursionError): # RecursionError: nesting too deep for the parser | |
| except (yaml.YAMLError, RecursionError, ValueError): # invalid YAML, excessive nesting or oversized integer |
There was a problem hiding this comment.
Fixed in 21e287c: the frontmatter parser's fallback catches ValueError too (yaml raises it for an integer over the digit limit), so an oversized scalar unrelated to paths no longer crashes budget. Pinned by oversized-integer-unrelated-to-paths.
| values = [m["inline"]] + [line.split("-", 1)[1] for line in m["items"].splitlines() if line.strip()] | ||
| return any(v.strip().strip("[]").strip(" \t\"'") for v in values) |
There was a problem hiding this comment.
Must-fix: the malformed-YAML fallback treats comments as patterns and stops collecting list items at comment lines. For example, paths: # none yet\nfoo: [unclosed\n returns True despite naming no pattern, whereas paths:\n # the api\n - "src/**"\nfoo: [unclosed\n returns False despite naming one. Root rules therefore land in the wrong start/lazy totals. Make fallback extraction ignore YAML comments (without stripping quoted # characters) and allow blank/comment lines between list items; add both malformed-frontmatter cases to the parametrized tests.
There was a problem hiding this comment.
Fixed in 21e287c: when the frontmatter does not parse, the paths fallback cuts YAML comments from each value (a # that starts a word, outside quotes, so "src/#x" keeps it) and skips comment and blank lines between items. paths: # none yet is start; a comment line before an item is lazy. Pinned by the unparsed-* cases of test_only_a_paths_value_that_names_a_pattern_scopes_a_rule.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #326 +/- ##
==========================================
- Coverage 94.69% 94.66% -0.03%
==========================================
Files 332 341 +9
Lines 43409 44824 +1415
Branches 7380 7677 +297
==========================================
+ Hits 41104 42432 +1328
- Misses 1468 1522 +54
- Partials 837 870 +33
🚀 New features to boost your workflow:
|
…ng time A root rule that links to a guidance file (or to a file a root CLAUDE.md turns off) is the same bytes: the guidance entry now takes the earlier of the two loading conditions instead of the rule being dropped, so the file is counted at start and no longer warned as unread. Frontmatter that fails to parse no longer crashes budget on an oversized integer (ValueError), and the fallback that reads the paths key ignores YAML comments and blank lines between items. Quoted # is kept. Thread: PRRT_kwDOVBg7g86ySxKH Thread: PRRT_kwDOVBg7g86ySxKK Thread: PRRT_kwDOVBg7g86ySxKP Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
Nothing to change in this review itself: it carries no finding of its own. Its five findings are the threads above, and each has its answer there. |
|
This summary restates the five threads above, and claims nothing of its own. Each finding has its answer in its thread. |
|
The report is for 82fd9bf and names neither line: one missing line and one partial branch in budget.py. Each branch this round adds runs in a regression case (listed in the thread replies). If the report on the new head still flags a line, I will cover it in the next round. |
| if target in by_target and rank[when] > rank[by_target[target]["when"]]: | ||
| by_target[target]["when"] = when |
There was a problem hiding this comment.
Must-fix: promoting an off guidance entry changes only when, retaining the full _chars() count even though it is loaded exclusively as a rule, whose frontmatter is stripped. For example, use CLAUDE.md = "claude\n", AGENTS.md = "---\npaths: []\n---\nagents\n", and .claude/rules/agents.md symlinked to AGENTS.md. Startup is charged for the frontmatter as well as the body. Reconcile the character count with the loading route when promoting an off entry, and extend the off-AGENTS alias test with frontmatter. Also consider lazy guidance promoted to start through a rule: its full guidance text should not all be charged at startup.
There was a problem hiding this comment.
Fixed in cc17f0d. A guidance file a rule links to takes the rule's count when the rule loads it earlier, since the rule route strips the frontmatter. In your example AGENTS.md is 7 characters at start (agents\n), not 25, and the warning is gone. Pinned by test_a_root_rule_linked_to_an_off_agents_md_is_charged_its_body. A lazy nested file promoted to start through a rule is charged its body: the rule loads the whole body at start, and nothing else of the file.
| counted.add(target) | ||
| if warning: | ||
| warnings.append(warning) | ||
| out.append({"path": f, "kind": "rule", "when": when, "chars": chars}) |
There was a problem hiding this comment.
Must-fix: by_target contains only guidance entries, so duplicate rule targets do not receive the loading-condition reconciliation. For example, create an unscoped !sub/.claude/rules/r.md and a root .claude/rules/r.md symlink to it. The nested path sorts first and produces a lazy entry; the root alias is then skipped because its target is counted but absent from by_target. The result reports zero startup characters despite the unconditional root rule. Add newly emitted rule entries to the target-to-entry map and test a nested alias that sorts before its root alias.
There was a problem hiding this comment.
Fixed in cc17f0d. A rule entry now goes in the target map, so a root alias that sorts after a nested real file upgrades that entry to start instead of being skipped. Your case (!sub/.claude/rules/r.md with a root symlink to it) reports 10 characters at start, not 0. Pinned by test_a_root_rule_linked_to_a_nested_rule_that_sorts_first_still_loads_at_start.
| values = _fallback_paths(frontmatter) | ||
| if values is None: | ||
| return False | ||
| return any(v.strip().strip("[]").strip(" \t\"'") for v in values) |
There was a problem hiding this comment.
Must-fix: the malformed-YAML fallback does not preserve the empty paths cases handled by the normal parser. For example, ---\npaths: null\nunused: [unclosed\n---\nbody\n is classified as lazy: the fallback returns "null", which this expression treats as a pattern. Without the unrelated malformed unused field, the same paths: null is correctly start. Similarly, paths: ["", ""] becomes scoped only when unrelated YAML is malformed. Normalize null and parse empty inline list items in the fallback so unrelated malformed metadata does not change the loading condition; add regression cases alongside the existing malformed-frontmatter tests.
There was a problem hiding this comment.
Fixed in cc17f0d. The unparsed-YAML fallback reads null, ~ and empty items as no pattern, so paths: null and paths: ["", ""] stay start even when unrelated metadata is malformed. Pinned by unparsed-null-unrelated-malformed and unparsed-empty-items-unrelated-malformed in the parametrized test.
vlad-perevezentsev
left a comment
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at 21e287c. Counting .claude/rules closes a real gap in start, and the loading rules match what the PR measured. One problem should be fixed before merge: paths values go through str(), so a short alias chain in a rule's frontmatter hangs repo budget. The rest are smaller edge cases, in the line comments.
Agreed with @exai-bot: a rule-to-rule link that sorts first keeps lazy; a guidance file promoted through a rule is charged its frontmatter; the YAML fallback reads paths: null and ["", ""] as patterns. On the open question about a rule that a CLAUDE.md imports, I replied in its thread: keep the code and fix the docs.
What I ran, at 21e287c:
make ci-local --offline: 0 failed, 19 passed, 13 skipped (network and Docker jobs only).- Scratch repros with the PR's
budget(): the alias chain (343 bytes 0.6 s, 393 bytes 4.3 s, 10x per level),paths: true/5/{a: 1}, a BOM rule,.claudeas a directory link, a rule-to-rule link, a danglingnotes.txtlink, and an imported rule with its own@path.
Checked, no issues
- Earlier threads: the
ValueErrorfallback, comments in the unparsedpaths, and a root rule linked to an off AGENTS.md or a nested CLAUDE.md are fixed, and each has a test. - Schemas:
repo_budget.v1only addsruletokindandharness_profile.v1only adds an optionalclaude_rules, so both widen in place; the Codex and Copilot profiles leave rules off. - Changelog: the fragment says that
startwas too low by every root rule before, so earlier budgets are not comparable with new ones.
| if isinstance(meta, dict): | ||
| value = meta.get("paths") | ||
| items = value if isinstance(value, list) else str(value or "").split(",") | ||
| return any(str(v).strip() for v in items if v is not None) |
There was a problem hiding this comment.
_has_paths calls str() on whatever YAML gives for paths, so a few hundred bytes of YAML aliases make repo budget hang or run out of memory.
yaml.safe_load keeps aliases as shared references, so it is fast, but str(v) on a list item (or on a dict value of paths) expands them. Repro with the PR code: a rule whose frontmatter is a0: &a0 [x,x,...] (10 items), then a1: &a1 [*a0,*a0,...] and so on, ending with paths: [*aN]. 343 bytes (7 levels) take 0.6 s, 393 bytes (8 levels) 4.3 s, and each level costs 10x more time and memory, so 10 levels is minutes and gigabytes. repo budget reads repositories you do not control.
The same str() also makes paths: true, paths: 5 and paths: {a: 1} scope the rule (all give lazy), while false and null give start; nothing measured says Claude Code reads such a value as a pattern.
Suggestion: only count strings, and never str() a container:
items = value if isinstance(value, list) else [value]
return any(isinstance(v, str) and v.strip() for v in items)(with the comma split for an inline string kept as now), plus a test with a nested-alias paths and one with paths: true.
There was a problem hiding this comment.
Fixed in cc17f0d, with your suggestion. A paths value is read as strings only and is never str()-ed, so an alias chain no longer expands: a 6-level chain took 0.66 s on head and parses at once now. Pinned by test_a_paths_value_is_never_stringified, which fails if a container is converted to text. Side effect: paths: true, paths: 5 and paths: {a: 1} now count as naming no pattern (start), as suggested. Nothing measured says Claude Code reads them as a pattern; a probe that shows otherwise would be a follow-up. Pinned in the parametrized test.
| text = tree.read_text(path) | ||
| except (OSError, ValueError, RuntimeError) as err: # UnicodeDecodeError is a ValueError; RuntimeError a loop | ||
| return 0, False, f"{path}: not readable ({_unreadable_reason(err)}), counted as 0" | ||
| m = _FRONTMATTER.match(text) |
There was a problem hiding this comment.
A rule that starts with a UTF-8 BOM is read as having no frontmatter, so its paths is ignored.
_FRONTMATTER is anchored at \A---. .claude/rules/r.md = "\ufeff---\npaths: src/**\n---\nbody\n" gives start with 28 chars, instead of lazy with 5. dokimi.utils.parse_frontmatter, which _skills in this module uses for SKILL.md, strips the BOM first, so the same frontmatter in a skill is parsed. A rule saved by a Windows editor is enough to move it into start.
Suggestion: strip a leading \ufeff before matching (and count the body after it), as parse_frontmatter does; one test case with a BOM.
There was a problem hiding this comment.
Fixed in cc17f0d. A leading UTF-8 BOM is stripped before the frontmatter match, as parse_frontmatter does for SKILL.md. Your example is now lazy with 5 characters. Pinned by test_a_rule_with_a_utf8_bom_reads_its_frontmatter.
| try: | ||
| if tree.is_dir(tree.resolve(f, dirs=True)): # a tree lists a link to a directory as one entry | ||
| warnings.append(f"{f}: a link to a directory; the rules behind it are not counted") | ||
| except (OSError, ValueError, RuntimeError): # out of the tree (a shared rules dir), dangling or a loop |
There was a problem hiding this comment.
Nit: A dangling or out-of-tree link to a non-Markdown file under .claude/rules warns that "any rules behind it are not counted".
.claude/rules/notes.txt -> nowhere.txt gives that warning, and repo budget --fail-on-warning fails a CI gate on a file that can never be a rule (a regular notes.txt gives none). Suggestion: in the except branch, warn only when the name has no suffix (it may be a directory link) and skip other suffixes.
There was a problem hiding this comment.
Fixed in cc17f0d. A dangling or out-of-tree link with a suffix (notes.txt) is no longer warned, since no rule can be behind it. A name without a suffix is still warned, because it may be a directory link. Pinned by test_a_dangling_link_to_a_non_markdown_file_under_rules_is_not_warned. A dangling .md rule is still named and counted as 0, pinned by test_a_dangling_link_to_a_markdown_rule_is_named_and_counted_as_zero.
| hop limit, each of which also trips | ||
| `--fail-on-warning`. `start` is not an upper bound: it is too low by what it does not count | ||
| (`.claude/rules/*.md` files; only `@path.md` imports are followed, so `@notes.txt` adds nothing; | ||
| `--fail-on-warning`. For a profile with `guidance.claude_rules` (Claude Code), |
There was a problem hiding this comment.
Nit: The list of warnings that "each ... trips --fail-on-warning" just above does not name the new one, a link under .claude/rules to a directory or out of the tree. The module docstring now says six checks. Suggestion: add it to that list.
There was a problem hiding this comment.
Fixed in cc17f0d. The list of checks that trip --fail-on-warning in architecture.md now names the link under .claude/rules to a directory or out of the tree.
… the harness does - A rule with a UTF-8 BOM has its frontmatter read, as a SKILL.md does. - `paths` is a pattern only when it is a string or a list of strings; the value is never str()-ed, so a YAML alias chain cannot expand into a hang. Non-string values (true, 5, a mapping) name no pattern. - The unparsed-YAML fallback treats null, ~ and empty items as no pattern, so unrelated malformed metadata does not change the loading condition. - A rule linked to a file already counted takes the earlier loading condition and that route's count (a rule's frontmatter is removed before it loads). - A rule entry joins the target map, so a root alias that sorts after a nested real file still promotes it to start. - A dangling link to a suffixed non-Markdown file under .claude/rules is not warned; a name without a suffix still is (it may be a directory link). - architecture.md lists the rules-link warning among the checks that trip --fail-on-warning. Thread: PRRT_kwDOVBg7g86yUV4N Thread: PRRT_kwDOVBg7g86yUV4R Thread: PRRT_kwDOVBg7g86yUV4U Thread: PRRT_kwDOVBg7g86yVf4V Thread: PRRT_kwDOVBg7g86yVf4Y Thread: PRRT_kwDOVBg7g86yVf4b Thread: PRRT_kwDOVBg7g86yVf4c Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| values = _fallback_paths(frontmatter) | ||
| if values is None: | ||
| return False | ||
| return any(_names_pattern(v) for v in values) |
There was a problem hiding this comment.
Must-fix: the fallback does not preserve the paths classification when unrelated metadata is malformed. For example, paths: true\n is unscoped, but paths: true\nunused: [unclosed\n becomes scoped because _names_pattern treats true as a string pattern; numbers and mappings have the same problem. Conversely, paths: [\n "src/**"]\nunused: [unclosed\n becomes unscoped because _fallback_paths collects only [ and stops before the pattern. These produce incorrect start/lazy totals. Parse the isolated paths value (including flow-list continuation lines) while retaining the documented non-string handling, and add paired tests with and without unrelated malformed YAML.
There was a problem hiding this comment.
Fixed in f2da52f. A paths key that the rest of the frontmatter does not parse is now read on its own, with its flow-list continuation lines, so paths: true stays unscoped and a paths: [ split over lines scopes the rule. Pinned by four new cases in test_only_a_paths_value_that_names_a_pattern_scopes_a_rule (new ones red before).
There was a problem hiding this comment.
🟡 Changes recommended
Directory symlink handling can silently omit rules or report fictitious entries, while rule imports are still expanded despite the no-follow contract.
3 open issues
What changed in this PR
3 new findings.
This PR extends repo budget to include Claude Code rules, reducing undercounting of repository context.
Changes:
- Counts rule bodies as startup or lazy context, with import deduplication and link warnings.
- Enables rules through an optional harness-profile setting and widens the budget schema.
- Adds regression tests and updates documentation and release notes.
| File | Description |
|---|---|
dokimi/tests/test_repo_budget_schema.py |
Validates artifacts containing rule entries. |
dokimi/tests/test_repo_budget_rules.py |
Tests rule classification, parsing, imports, and links. |
dokimi/dokimi/schemas/repo_budget.v1.json |
Adds the rule entry kind. |
dokimi/dokimi/schemas/harness_profile.v1.json |
Allows the optional claude_rules setting. |
dokimi/dokimi/repos/profiles/claude-code.yaml |
Enables rule counting for Claude Code. |
dokimi/dokimi/repos/harness_profile.py |
Exposes the profile’s rule-counting setting. |
dokimi/dokimi/repos/budget.py |
Implements rule discovery, counting, and warnings. |
dokimi/dokimi/commands/repo.py |
Passes the profile setting into budget calculation. |
dokimi/docs/what-it-measures.md |
Includes rules in the budget description. |
dokimi/docs/repo-evaluation-contract.md |
Documents rule-loading conditions and limitations. |
dokimi/docs/architecture.md |
Describes rule accounting and link warnings. |
dokimi/changelog.d/20261030_180000_napetrov_budget-claude-rules.md |
Announces rule support and schema changes. |
Review effort: Balanced
| unreadable_warnings.append(warning) | ||
| entries.append({"path": f, "kind": "guidance", "when": when, "chars": chars}) | ||
| counted = {_target(tree, f) for f in guidance} | ||
| entries += _rules(tree, files if rules else [], counted, start_t, unreadable_warnings, entries) |
There was a problem hiding this comment.
Fixed in f2da52f. An @ path in a rule is not followed, also when a CLAUDE.md imports the rule (matches docs/repo-evaluation-contract.md). A file named CLAUDE.md under .claude/rules is a rule, not guidance. Pinned by test_a_rule_does_not_expand_its_at_imports_even_when_claude_md_imports_it and test_a_file_named_claude_md_under_rules_is_a_rule_not_guidance.
| if not PurePosixPath(f).suffix: # a file with a suffix (notes.txt) is never a rule; a name without one may be a directory | ||
| warnings.append(f"{f}: a link that does not resolve in the tree; any rules behind it are not counted") |
There was a problem hiding this comment.
Fixed in f2da52f. A link under .claude/rules that does not resolve is now named whatever its suffix: the tree cannot tell a dangling file from an external directory. The old test that stayed silent on notes.txt is replaced by test_a_dangling_link_under_rules_that_is_not_markdown_is_named.
| for f in sorted(f for f in files if _RULES_ENTRY.match(f) and not RULE_GLOB.match(f)): | ||
| try: | ||
| if tree.is_dir(tree.resolve(f, dirs=True)): # a tree lists a link to a directory as one entry | ||
| warnings.append(f"{f}: a link to a directory; the rules behind it are not counted") | ||
| except (OSError, ValueError, RuntimeError): # out of the tree (a shared rules dir), dangling or a loop | ||
| if not PurePosixPath(f).suffix: # a file with a suffix (notes.txt) is never a rule; a name without one may be a directory | ||
| warnings.append(f"{f}: a link that does not resolve in the tree; any rules behind it are not counted") | ||
| for f in sorted(f for f in files if RULE_GLOB.match(f)): |
There was a problem hiding this comment.
Fixed in f2da52f. An entry that resolves to a directory is named as a linked directory before it is classified as a rule, so shared.md no longer yields a zero-character rule. Pinned by test_a_linked_directory_named_like_a_rule_is_named_not_counted_as_a_rule.
…e import walk, names unresolved rule links - A `paths` key that the rest of the frontmatter fails to parse is read as its own block, with its flow-list continuation lines, so `paths: true` and `paths: [` over several lines classify the same as the parsed key (Thread: PRRT_kwDOVBg7g86yWM2k). - An @ path in a rule is not followed, also when a CLAUDE.md imports the rule, and a file named CLAUDE.md under .claude/rules is a rule, not guidance (Thread: PRRT_kwDOVBg7g86yWNjN). - A link under .claude/rules that does not resolve is named whatever its suffix, since the tree cannot tell a dangling file from an external directory (Thread: PRRT_kwDOVBg7g86yWNjn). - An entry that resolves to a directory is named as a linked directory before it is classified as a rule, also for a name ending in .md (Thread: PRRT_kwDOVBg7g86yWNj0). Thread: PRRT_kwDOVBg7g86yWM2k Thread: PRRT_kwDOVBg7g86yWNjN Thread: PRRT_kwDOVBg7g86yWNjn Thread: PRRT_kwDOVBg7g86yWNj0 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| names = set(guidance_names) | ||
| every = [f for f in files if f.rsplit("/", 1)[-1] in names] | ||
| # a file under .claude/rules is a rule whatever its name (a rules/CLAUDE.md is not guidance) | ||
| every = [f for f in files if f.rsplit("/", 1)[-1] in names and not RULE_GLOB.match(f)] |
There was a problem hiding this comment.
Must-fix: Apply this exclusion only when rules is enabled. With the Codex profile (reads_claude_rules == False), a tree containing .claude/rules/AGENTS.md now loses that recognized guidance file entirely: it is excluded here and _rules receives no files. The unconditional RULE_GLOB check in _imports likewise truncates explicit import chains for profiles that do not read Claude rules. Gate both special cases on the profile flag and add disabled-rules regression tests; the existing profile test only verifies that no rule entries appear.
There was a problem hiding this comment.
Fixed in 6cc0717: the skip of .claude/rules files (as rules, and in _imports) now applies only when the profile reads rules.
For codex, .claude/rules/AGENTS.md counts as guidance and its @imports are followed.
Test: test_a_profile_that_does_not_read_rules_still_counts_a_rules_file_as_guidance_and_imports (red on f2da52f).
| if not item.strip() or item[:1] in (" ", "\t", "-", "#"): | ||
| block.append(item) | ||
| else: | ||
| break |
There was a problem hiding this comment.
Must-fix: The fallback drops unindented flow-list continuation lines. For frontmatter paths: [\n"src/**"]\nunused: [unclosed\n, parsing the full document fails on unused, but the paths value itself names a pattern. This loop stops before "src/**"], leaving only paths: [; _names_pattern then returns false and the rule is incorrectly charged at start. Track open flow collections when collecting the block, and add a malformed-unrelated-field test with an unindented continuation (the current tests only cover indented continuations).
… keeps its continuation A profile that does not read .claude/rules (codex) still counts a file there as guidance and follows its @imports. An unindented continuation line of a flow list that the paths key opens is part of the key's block. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| for i, line in enumerate(lines): | ||
| if _PATHS_LINE.match(line): | ||
| block = [line] | ||
| depth = line.count("[") - line.count("]") # a flow list open on the key runs on, unindented or not |
There was a problem hiding this comment.
Must-fix: Brackets in YAML comments are counted as flow-list delimiters. For frontmatter paths: [] # [optional]\nunused: [unclosed\n (with an unmatched [ in the comment, e.g. change the comment to # [optional), full-document parsing fails on unused; this depth calculation then consumes that unrelated key into the paths block. Parsing the block fails too, and the text fallback treats unused: [unclosed as a pattern, reporting the rule as lazy despite paths: []. Ignore comments when tracking flow-list depth, including continuation lines, and add a regression test for an empty paths list with a bracket-bearing comment and unrelated malformed YAML.
| if rules and RULE_GLOB.match(f): | ||
| continue # an @ path in a rule is not followed (docs/repo-evaluation-contract.md) |
There was a problem hiding this comment.
Must-fix: This check only recognizes the import's spelling, not its resolved target. With CLAUDE.md containing @docs/memory/alias.md, docs/memory/alias.md symlinked to .claude/rules/r.md, and that rule containing @../../extra.md, _imports reads the alias and incorrectly adds an existing extra.md as a start import. _rules already deduplicates the alias against the rule by resolved target, so the output treats this as a rule while still expanding its imports, contrary to the new contract. Check both the literal path and _target(tree, f) for rule membership, and add an outside-rules alias regression test.
Suggestion:
| if rules and RULE_GLOB.match(f): | |
| continue # an @ path in a rule is not followed (docs/repo-evaluation-contract.md) | |
| if rules and (RULE_GLOB.match(f) or RULE_GLOB.match(_target(tree, f))): | |
| continue # an @ path in a rule is not followed (docs/repo-evaluation-contract.md) |
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 6cc0717. The new rules accounting holds up against the cases I tried. Two risks remain, in the malformed-YAML fallback and in import traversal through a link; both reproduce at this head. One nit on the docs.
What I ran, on a Linux review host at 6cc0717:
make ci-local: 28 passed, 2 failed (gitleaks download returned HTTP 504, so its scan did not run), 7 skipped; 12,019 tests; changed-line coverage 94%.- Reproductions for the findings below, on trees built the way
tests/test_repo_budget_rules.pybuilds them. - Mutation check on the fallback: dropping
"null"and"~"from_names_patternstill passes all 49 tests intest_repo_budget_rules.py. Those items only reach_names_patternwhen thepathskey itself does not parse, so the cases that are named as pinning them go through the YAML parse of the key instead.
Checked, no issues
- Schemas:
ruleadded to thekindenum and optionalclaude_rulesadded to the profile schema both widen in place; the schema compatibility check passes against the base. - Profiles: Codex and Copilot leave
claude_rulesoff, and.claude/rules/CLAUDE.mdis then plain guidance. - Dedup and links: a root alias of a nested rule, a rule linked to an off AGENTS.md, directory links and dangling links each give one entry and the expected warning.
- Commits: all eight non-merge commits carry a
Signed-off-by.
Bot points I checked that no longer apply at this head (13)
- Copilot's and exai-bot's point on an imported rule's
@paths being followed does not apply: at this head_importsstops at a rule, and a test pins thatextra.mdis not charged. - exai-bot's point on a root rule linked to an off AGENTS.md losing startup context does not apply: the guidance entry takes the earlier loading condition, and tests pin it.
- exai-bot's point on an oversized integer crashing the frontmatter parser does not apply:
ValueErroris caught in_has_paths. - exai-bot's point on comments in the unparsed
pathsfallback does not apply:_without_commentcuts them and comment lines between items are skipped. - exai-bot's point on a promoted guidance entry keeping its frontmatter does not apply: the entry takes the rule's count when the rule loads it earlier.
- exai-bot's point on duplicate rule targets not being reconciled does not apply: rule entries are added to the target map and the nested-first case loads at start.
- exai-bot's point on
paths: nulland empty items in the fallback does not apply:_names_patternreadsnull,~and empty items as no pattern. - exai-bot's point on
paths: trueand a split flow list in the fallback does not apply: the key is parsed on its own with its continuation lines. - Copilot's point on a CLAUDE.md-imported rule expanding its imports and on a
rules/CLAUDE.mdfile does not apply: both are handled at this head and tested. - Copilot's point on an unresolved external link with a suffix suppressing the warning does not apply: any non-Markdown entry that does not resolve is named.
- Copilot's point on a directory link named like a rule does not apply: an entry that resolves to a directory is named before it is classified.
- exai-bot's point on gating the rules exclusion on the profile flag does not apply: both the exclusion and the
_importsskip are gated onrules, and a test covers the Codex profile. - exai-bot's point on unindented flow-list continuation lines does not apply:
_paths_blockkeeps them while a flow list is open.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to b1f2624, which fix the nit from my review:
b1f2624:dokimi/docs/repo-evaluation-contract.md:51: added that a rule a root CLAUDE.md or AGENTS.md imports isstartwhatever itspaths, with the same sentence inarchitecture.mdand the changelog fragment.
make ci-local at b1f2624: 30 passed, 0 failed, 7 skipped.
Not approving yet. Still open: the two risks from my review.
…s or imports read A bracket inside a YAML comment on the paths line (or on a comment line under it) counted toward the flow-list depth, so an unrelated malformed key below was pulled into the paths block and an empty paths was read as scoped. Comments are now left out of the depth. An @ import of an alias that links to a rule was still followed, though an @ path inside a rule is not. The rule check now also looks at the file the import resolves to, as the rule dedup already does. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| if s and not s.startswith("#"): | ||
| values.append(_without_comment(s[1:] if s.startswith("-") else s)) |
There was a problem hiding this comment.
Must-fix: For a root rule containing ---\npaths: []\n unused: [unclosed\n---\nbody\n, _paths_block() includes the indented unused line. Both YAML parses fail, and this fallback treats unused: [unclosed as a paths pattern, reporting the rule as lazy even though paths: [] names no pattern. This drops the body from the startup total. Keep unrelated indented fields out of the fallback pattern values (including after a complete inline value), and add this case to the parametrized frontmatter tests with when="start" and body-only character counting.
There was a problem hiding this comment.
Fixed in adbfd73. An indented line that is a mapping key ( unused: [unclosed) now ends the paths block when no flow list is open, so it is no longer read as a pattern. Your example (paths: [] plus that line) now loads at start with body-only counting.
Tests: four new cases in the frontmatter table. The paths: [] and paths: null ones are red on cd427e9; the two after a pattern and after a list pass on both (guards).
Local: test_repo_budget_rules.py + test_repo_budget.py 137 passed; full suite rc 0; ruff clean. Self-review: 0 blocking.
When the frontmatter does not parse, the paths key was read with its indented continuation lines, so ` unused: [unclosed` under `paths: []` counted as a pattern and the rule was reported as lazy. A line that is an indented mapping key now ends the block, outside an open flow list. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…) under the growth cap _imports kept one origin per file, the first root that reached it. A second root that reached the same file later (through a shared import) was dropped, so a cut import that only that root's chain reaches went unwarned. Roots are now carried along the recorded import edges to a fixed point. budget() measured 19 against its baseline of 18. Its guidance, cut-warning and skill steps move to helpers; the baseline entry is removed, not raised. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| if not path or path in seen or _target(tree, path) in reached: | ||
| if not path: | ||
| continue | ||
| edges.append((_target(tree, f), _target(tree, path))) |
There was a problem hiding this comment.
Must-fix: root provenance is propagated across imports beyond the hop limit, including imports skipped because another root already reached their target. For example, use sub/CLAUDE.md: @mid.md, sub/mid.md: @.claude/rules/r.md, zother/CLAUDE.md: @bridge.md, zother/bridge.md: @../sub/mid.md, and an unscoped sub/.claude/rules/r.md. With import_max_depth=1, rules=True, only sub's chain reaches mid within the limit, so its cut rule import should be quiet. However, the recorded bridge→mid edge makes _roots_reaching() attach zother to mid too, producing a spurious warning and failing --fail-on-warning. Track bounded reachability per root when propagating provenance; recording all edges and taking an unrestricted fixed point loses the hop-limit constraint. Add this unequal-length shared-chain case as a regression test.
There was a problem hiding this comment.
Fixed in 5d6358e: each root is walked over the import edges within the hop limit, so a root past the limit is no longer named.
Test: test_a_cut_import_is_not_blamed_on_a_root_that_reaches_the_shared_file_past_the_hop_limit.
| if path in seen: | ||
| continue | ||
| t = _target(tree, path) | ||
| done = reached_by_rule if RULE_GLOB.match(path) else reached |
There was a problem hiding this comment.
Must-fix: gate the separate rule-route tracking on rules. With rules=False, put @.claude/rules/link.md\n@extra.md\n in root AGENTS.md, symlink .claude/rules/link.md to ../../AGENTS.md, and create both extra.md and .claude/rules/extra.md. The root's canonical target is already in reached, but this branch checks the empty reached_by_rule instead and expands AGENTS.md again relative to .claude/rules/. Consequently, .claude/rules/extra.md becomes a spurious start-time import and inflates the budget. Previously the canonical-target cycle check stopped this traversal. Keep a single reached set when rules are disabled and add a regression test for this alias cycle.
Suggestion:
| done = reached_by_rule if RULE_GLOB.match(path) else reached | |
| done = reached_by_rule if rules and RULE_GLOB.match(path) else reached |
There was a problem hiding this comment.
Fixed in 5d6358e: with rules off, a rule-named link shares the one reached set, so the alias is not expanded again.
Test: test_an_alias_cycle_back_to_agents_md_is_not_expanded_again_when_rules_are_off.
…ute on rules A cut import is named for every guidance root whose chain reaches the importing file within the hop limit. The provenance was carried along all recorded edges, so a root that reached a shared file only past the limit was blamed for the cut. Each root is now walked over the edges with the same limit. With rules off, a rule-named link shares the one reached set, so an alias back to an already-read AGENTS.md is not expanded again. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
| rule = rule_entries.get(t) | ||
| if rule is None or RULE_GLOB.match(f) or RULE_GLOB.match(t) or (when == "lazy" and rule["when"] == "start"): | ||
| continue |
There was a problem hiding this comment.
Must-fix: rule-path imports through a linked rules directory still retain frontmatter when their target is a guidance entry. Reproducer: .claude/rules -> ../shared, CLAUDE.md = '@.claude/rules/AGENTS.md\n', and shared/AGENTS.md = '---\npaths: []\n---\nbody\n'; call budget(tree, NAMES, rules=True). The guidance loop marks shared/AGENTS.md as start and counts all 23 characters. Discovery only sees the directory link, so it cannot reconcile that count, and this branch skips the import because its target is counted but absent from rule_entries. The sole loading route strips frontmatter in _import_text, so this entry should count the 5-character body. Reconcile rule-only imported guidance targets here as well, and add a regression alongside the linked-rules-directory tests.
There was a problem hiding this comment.
Fixed in 5ade9e1: a rules dir that is itself a link now updates the guidance entry its rule link reaches (start count, no frontmatter). Test: test_a_rule_import_through_a_linked_rules_directory_counts_the_body_of_the_guidance_it_reaches.
| if any(a in claude for a in aliases) or not (displaced and f.rsplit("/", 1)[-1] == "AGENTS.md"): | ||
| own.add(t) |
There was a problem hiding this comment.
Must-fix: this marks nested guidance as independently loaded without retaining its loading time. Reproducer: CLAUDE.md = '@.claude/rules/nested.md\n', sub/CLAUDE.md = '---\npaths: []\n---\nnested\n', and .claude/rules/nested.md symlinked to ../../sub/CLAUDE.md; call budget(tree, NAMES, rules=True). _guidance_when() already labels the nested guidance start because the rule import reaches it, while this branch puts its target in own_t merely because it is CLAUDE.md. The equal-rank reconciliation in _rules() consequently retains the full 25-character file instead of the 7-character body. Its independent memory route loads only lazily; the startup route strips frontmatter. Track the loading time of independent/full-text routes when reconciling counts, and add this imported-rule-link case (also applicable to active nested AGENTS.md).
There was a problem hiding this comment.
Fixed in 5ade9e1: a nested own full-text route that is lazy no longer wins over a rule route at the same time, so the rule's start time and body count are kept. Test: test_a_root_import_of_a_rule_linked_to_a_nested_claude_md_counts_its_body_at_start.
| entry["when"] = when | ||
| entry["chars"] = chars |
There was a problem hiding this comment.
Must-fix: promoting a scoped rule alias while retaining kind='guidance' loses information the receipt needs. Reproducer: root CLAUDE.md = 'root\n', sub/AGENTS.md = '---\npaths: other/**\n---\nagents\n', and .claude/rules/agents.md symlinked to ../../sub/AGENTS.md. With rules enabled, this changes the displaced AGENTS.md from off to lazy. Passing that budget into build_receipt() for the existing SUB_ROW (read=sub/REACH_PROBE.txt) produces expected: true: _own_guidance() treats sub/AGENTS.md as independent guidance, and _expected() assumes all such lazy entries load in their subtree. But this file loads only through a rule whose pattern does not match that read. Preserve rule-only loading provenance, or make the receipt exclude rule-promoted guidance from its independent-subtree inference and return unknown when scope cannot be established. Add a real-budget receipt regression for this scoped alias.
There was a problem hiding this comment.
Fixed in 5ade9e1: _own_guidance no longer marks a file that a rule links to as expected in its subtree run, so the receipt says unknown there. The budget keeps lazy. Test: test_a_scoped_rule_alias_of_a_nested_agents_md_is_not_expected_in_its_subtree.
| if not path or path in seen or _target(tree, path) in reached: | ||
| if not path: | ||
| continue | ||
| edges.append((_target(tree, f), _target(tree, path))) |
There was a problem hiding this comment.
Must-fix: canonicalizing the provenance graph merges distinct relative-import routes. Reproducer: sub/CLAUDE.md = '@.claude/rules/link.md\n', sub/.claude/rules/link.md -> ../../../shared.md, shared.md = '@r.md\n', zother/CLAUDE.md = '@../shared.md\n', and an unscoped sub/.claude/rules/r.md (no root r.md), with import_max_depth=1, rules=True. Both routes expand separately, but _roots_reaching() assigns both roots to the canonical shared.md node. Consequently the cut of sub/.claude/rules/r.md is incorrectly attributed to zother and warns, although that rule loads independently with its actual sub/ importer; --fail-on-warning then fails spuriously. Preserve route-specific provenance (including the importing path/relative base) through the bounded traversal and add this two-root alias regression.
There was a problem hiding this comment.
Fixed in 5ade9e1: cut provenance is keyed by the path each route is read at, so two routes to one target no longer merge. Test: test_a_cut_import_is_not_blamed_on_a_root_that_reaches_the_file_of_the_rule_route_by_another_path.
…by path - a rules dir that is itself a link is not listed by the tree; a rule link to guidance now updates that guidance entry (start or lazy) - a nested own full-text route that is lazy no longer wins over a rule route at the same time; the rule body counts from start - cut-import provenance is keyed by the path each route is read at, so two routes to one target no longer merge - receipt: a guidance file a rule links to is not expected in its subtree run (expected becomes unknown) Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
_import_entries scored 12 against the cap of 10. The branch that folds an import into a file already counted (a rule or guidance entry) moves to _counted_import; the loop keeps the new-file path. Behaviour unchanged. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
vlad-perevezentsev
left a comment
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at cb27212. The cut-warning case from my last review is fixed: a cut is now judged by the guidance file its chain starts from, and the receipt keeps start-up rules. Two small receipt points are in the line comments. The branch now conflicts with main in scripts/growth_baseline.json and needs a rebase.
What I ran, at cb27212:
make ci-local --offline: 1 failed, 18 passed, 13 skipped. The failure isschema_compatagainst currentmain:headroom.v1.jsonandrepo_environment.v1.jsonchanged onmainafter this branch's base, not in this PR.- Repros with the PR's
budget(): theother/CLAUDE.md -> sub/notes.mdchain now warns; the earlier cases (alias chain, BOM, rule-to-rule link, broken-YAMLpaths, root and nested cut warnings) still give the fixed results.
Checked, no issues
- Cut provenance: roots are walked within the hop limit and keyed by the path read, so a root past the limit or on another route is not blamed.
- Rules off: with
rules=Falsea rule-named link shares the one reached set, as before the PR.
| ruled = {_target(tree, f) for f in tree.files() if RULE_GLOB.match(f)} | ||
| return frozenset(p for p in guidance if p not in aliases and p not in imported and _target(tree, p) not in ruled) |
There was a problem hiding this comment.
ruled drops every file a rule links to from own, including a nested CLAUDE.md that loads in its subtree anyway.
With sub/CLAUDE.md and a scoped root rule .claude/rules/x.md -> ../../sub/CLAUDE.md, the read=sub run used to expect sub/CLAUDE.md; now it is unknown, so the receipt loses a check it could make. Only guidance that loads through the rule alone (a displaced AGENTS.md) needs to leave own.
Suggestion: exclude a ruled file only when budget has no route of its own for it, e.g. a displaced AGENTS.md.
There was a problem hiding this comment.
Done in f96e654: a nested CLAUDE.md a rule links to keeps its own check in its subtree; only a linked AGENTS.md is unknown there. Regression test added.
| target_of = {alias: item["path"] for item in reach["files"] for alias in item.get("aliases", [])} | ||
| return {target_of.get(e["path"], e["path"]): e["when"] | ||
| for e in budget["entries"] if e["kind"] in ("guidance", "import", "skill listing")} | ||
| for e in budget["entries"] if e["kind"] in ("guidance", "import", "rule", "skill listing")} |
There was a problem hiding this comment.
The receipt now lists start-up rules as sources, but its notice and the docs still say only start-up imports and skill listings are not probed.
NOT_PROBED (receipt.py:40) and repo-evaluation-contract.md:1173-1181 name imports and skill listings only, and the changelog fragment does not mention the receipt at all, though a rule-linked file can now flip from expected to unknown.
Suggestion: add rules to NOT_PROBED and to that section, and one line about the receipt to the fragment.
There was a problem hiding this comment.
Done in f96e654: NOT_PROBED and the contract doc now name start-up rules; changelog has a receipt line.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at cb27212. The cut-import provenance now holds up on the nested and shared-root cases I tried, and both earlier threads on it are fixed. One more risk is a rules-off gate that no test pins, and one earlier test-gap thread is still open on the same kind of line.
What I ran, on a Linux review host at cb27212:
make ci-local: 28 passed, 2 failed, 7 skipped. The two failures are the gitleaks install and the commit scan that needs it; a manual gitleaks scan of the 29 commits finds no leaks. Changed-line coverage gate passes.- Reproductions for the findings, with no model or network involved.
- Mutation check on the commits since 5fe9b15 (30 mutants): 12 pass the three budget and receipt test files; the two that change a total or a warning are listed below.
Still open from my earlier review:
dokimi/tests/test_repo_budget_rules.py:497: the cut-warning branches are pinned only withrules=True. Replacingnested_unscoped = _nested_unscoped(tree, files) if rules else set()(budget.py:157) with the unconditional call passes all 254 tests. Withsub/CLAUDE.md=@.claude/rules/r.md, an unscopedsub/.claude/rules/r.md,import_max_depth=0andrules=False, the head warnssub/CLAUDE.md: import of sub/.claude/rules/r.md is past the harness's 0-hop import limit, not followed; the mutant returns[], so--fail-on-warningwould pass over it. This is the mutant my thread listed. Add therules=Falsecounterpart oftest_a_cut_import_of_an_unscoped_nested_rule_does_not_warn, expecting the warning.
I agree with @vlad-perevezentsev's points on the receipt: _own_guidance drops a nested sub/CLAUDE.md that a rule links to, and NOT_PROBED and the contract do not name rules. New evidence: with sub/CLAUDE.md and .claude/rules/x.md linked to it, the read=sub run gives expected: unknown, and true without the link. It is a lost check, not a wrong one, so I count it as a nit.
I agree with @vlad-perevezentsev that the branch needs today's main merged in. New evidence: at main c17f630 it conflicts in budget.py, harness_profile.py, commands/repo.py and repo-evaluation-contract.md.
Bot points I checked that no longer apply at this head (41)
- exai-bot's point on the docs omitting a rule that guidance imports does not apply: the contract, architecture page and changelog now state the imported-rule case and its
startclass. - exai-bot's point on a rule's
@paths not being enforced does not apply: the maintainer chose to follow them for an imported rule, the docs say so, and tests pin it. - exai-bot's point on a root rule linked to an off AGENTS.md losing startup context does not apply: the guidance entry takes the earlier loading condition, and tests pin it.
- exai-bot's point on an oversized integer crashing the frontmatter parser does not apply:
ValueErroris caught and the rule falls back to itspathslines. - exai-bot's point on comments in the unparsed
pathsfallback does not apply: comments are cut and comment lines between items are skipped. - exai-bot's point on a promoted guidance entry keeping its frontmatter does not apply: the entry takes the rule's body count when the rule loads it earlier.
- exai-bot's point on duplicate rule targets does not apply: rule entries are in the target map, and a nested rule that sorts first still loads at start.
- exai-bot's point on
nulland empty items in the unparsedpathsfallback does not apply: they read as no pattern. - exai-bot's point on
paths: trueand a split flow list in the fallback does not apply: the key is parsed on its own with its continuation lines. - Copilot's point on a rule's
@paths and on arules/CLAUDE.mdfile does not apply: a file under.claude/rulesis a rule, and the imported-rule case is documented. - Copilot's point on an unresolved link with a suffix hiding the warning does not apply: every link under
.claude/rulesthat does not resolve is named. - Copilot's point on a directory link named like a rule does not apply: an entry that resolves to a directory is named before it is classified.
- exai-bot's point on gating the
.claude/rulesexclusion on the profile flag does not apply: the exclusion and the_importsskip depend onrules, and a Codex-profile test covers it. - exai-bot's point on unindented flow-list continuation lines does not apply: they stay in the
pathsblock while a list is open. - exai-bot's point on brackets in YAML comments does not apply:
_flow_depthignores them, with two tests. - exai-bot's point on an alias of a rule being import-expanded does not apply:
_importsmatches the resolved target, and a test pins it. - exai-bot's point on an indented field after
paths: []does not apply: an indented mapping key ends the block, with parametrized cases. - exai-bot's point on a scoped rule hiding a cut-import warning does not apply: only a
startrule counts as loaded, and a test pins the scoped case. - exai-bot's point on quoted keys after
paths: []does not apply: double- and single-quoted keys end the block, with cases. - exai-bot's point on a scoped alias of AGENTS.md hiding a cut import does not apply: independently loaded guidance is snapshotted before rule promotion, and a test pins it.
- exai-bot's point on warning for a dangling non-Markdown link under
.claude/rulesdoes not apply: the maintainer chose to warn on any dangling link there, and a test pins it. - exai-bot's point on unscoped nested rules and cut-import warnings does not apply: an unscoped nested rule imported from its own subtree is quiet, with tests for the scoped and out-of-subtree siblings.
- exai-bot's point on import order hiding what a rule link leads to does not apply: expansion is tracked per route kind, and a test covers both orders.
- exai-bot's point on an ordinary import losing frontmatter that a rule link removes does not apply: the ordinary import replaces the rule entry with its full count.
- exai-bot's point on frontmatter stripping when the profile does not read rules does not apply at the
_importslevel: the flag reaches all call sites and a rules-off test covers it. - exai-bot's point on a rule imported through a linked rules directory keeping its frontmatter does not apply: the 5-character body is counted, and a test pins it.
- exai-bot's point on an imported rule linked to off guidance keeping the full count does not apply: the body count is used, and a test pins it.
- exai-bot's point on
_importslosing its argument defaults does not apply:max_depth=None, rules=Falseare present at this head. - exai-bot's point on AGENTS.md import chains in the docs does not apply: the contract and architecture page name AGENTS.md as well as CLAUDE.md.
- exai-bot's point on the receipt's
budget()call ignoring the rules flag does not apply:repo.pypassesprofile.reads_claude_rules, and a test covers it. - exai-bot's point on a root cut hidden by a nested CLAUDE.md does not apply: the exemption skips pairs the start chain cuts, and a test pins it.
- exai-bot's point on an explicit import through a linked rules directory keeping its frontmatter does not apply: the body is counted, and a test pins it.
- exai-bot's point on an ordinary import of a rule behind a linked rules directory does not apply: the ordinary import replaces the body-only count in either order.
- exai-bot's point on the receipt dropping
ruleentries does not apply:_budget_whenincludesrule, and a test runs realbudget()output throughbuild_receipt(). - exai-bot's point on a cut import blamed on only the first root does not apply: provenance is kept per root, and a test pins it.
- exai-bot's point on root provenance past the hop limit does not apply: each root is walked within the limit, and a test pins it.
- exai-bot's point on gating rule-route tracking on
rulesdoes not apply: with rules off a rule-named link shares one reached set, and a test pins it. - exai-bot's point on a rule import through a linked rules directory keeping guidance frontmatter does not apply: the body count is used, and a test pins it.
- exai-bot's point on nested guidance reached by a rule import keeping its frontmatter does not apply: the rule's start time and body count win, and a test pins it.
- exai-bot's point on a scoped rule alias of nested AGENTS.md giving
expected: truein the receipt does not apply: such a file is no longer expected in its subtree run, and a test pins it. - exai-bot's point on merged provenance for two routes to one target does not apply: provenance is keyed by the path read, and a test pins it.
Resolve the budget.py, repo.py, harness_profile, contract and baseline conflicts; receipt keeps a nested CLAUDE.md's own check when a rule links to it (AGENTS.md linked from a rule is unknown in a subtree); NOT_PROBED names start-up rules; rules-off frontmatter pin test. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
budget() is under the complexity cap after the _import_entries split, so the ratchet asks for its entry to go. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at f96e654. The rules accounting holds up, and the delta since my last review is the merge of main plus one new test. One risk from my earlier review is still open: the cut-warning branches are pinned only with rules=True. One nit inline.
What I ran, on a Linux review host at f96e654:
make ci-local: 30 passed, 0 failed, 7 skipped; 13,127 tests; changed-line coverage 98%.- Reproductions for the findings, with no model or network involved.
- Mutation check on
budget.pyandreceipt.pyagainst the budget and receipt test files: therules andgate on the guidance-route recount is now killed; the mutants of the cut-warning branches listed below still pass.
Still open from my earlier review (@napetrov's cases in 738c454 do not kill these two mutants; I ran both against the head):
dokimi/tests/test_repo_budget_rules.py:511: two cut-warning mutants still pass all 278 tests intest_repo_budget_rules.py,test_repo_budget.py,test_repo_budget_nested.pyandtest_repo_receipt.py. (1)nested_unscoped = _nested_unscoped(tree, files) if rules else set()(budget.py:165) replaced by the unconditional call: withsub/CLAUDE.md=@.claude/rules/r.md, an unscopedsub/.claude/rules/r.md,import_max_depth=0andrules=False, the head warnssub/CLAUDE.md: import of sub/.claude/rules/r.md is past the harness's 0-hop import limit, not followed; the mutant returns[]. The new tests withrules=Falsecut at start, so this gate never decides there. (2)_target(tree, target) == tin_cut_is_quiet(budget.py:97) replaced byTrue: withsub/CLAUDE.md=@notes.md,sub/notes.md, an unscopedsub/.claude/rules/r.mdandimport_max_depth=0, the head warnssub/CLAUDE.md: import of sub/notes.md is past the harness's 0-hop import limit, not followed; the mutant returns[], so any cut import in a subtree with an unscoped rule goes quiet. Both decide whether--fail-on-warningfails. Tests to add: therules=Falsecounterpart oftest_a_cut_import_of_an_unscoped_nested_rule_does_not_warn, expecting the warning; and thesub/notes.mdtree above, expecting the warning.
Checked, no issues
- Delta: the new test pins the rules-off recount (
start == 52,AGENTS.md29 chars); removingrules andfrom that branch now fails it. - Merge with main: the
claude_displaces_agentscode and the rules code agree on a rule-linkedsub/AGENTS.md(start for an unscoped root rule, lazy for a scoped one, frontmatter stripped), a rules dir link, and.claude/rules/AGENTS.md. - Schemas and docs:
rulein thekindenum and optionalclaude_ruleswiden in place;schema_compatagainst the base passes; the contract, the architecture page and the changelog matchbudget()on the trees I tried. - Commits: all non-merge commits carry a
Signed-off-by.
Bot points I checked that no longer apply at this head (41)
- exai-bot's point on the docs omitting the imported-rule case does not apply: the contract, the architecture page and the changelog state it and its
startclass. - exai-bot's point on enforcing non-expansion of an imported rule's
@paths does not apply: the maintainer chose to follow them within the hop limit, the docs say so, and a test pins it. - exai-bot's point on a root rule linked to an off AGENTS.md losing startup context does not apply: the guidance entry takes the earlier loading condition, and tests pin it.
- exai-bot's point on an oversized integer crashing the frontmatter parser does not apply:
ValueErroris caught and thepathslines are read. - exai-bot's point on comments in the unparsed
pathsfallback do not apply: comments are cut and comment lines between items are skipped. - exai-bot's point on a promoted guidance entry keeping its frontmatter does not apply: it takes the rule's body count when the rule loads it earlier.
- exai-bot's point on duplicate rule targets without reconciliation do not apply: rule entries are in the target map, and a nested rule that sorts first still loads at start.
- exai-bot's point on
nulland empty items in the unparsedpathsfallback do not apply: they read as no pattern. - exai-bot's point on
paths: trueand a split flow list in the fallback do not apply: the key is parsed on its own with its continuation lines. - Copilot's point on a rule's
@paths and arules/CLAUDE.mdfile do not apply: a file under.claude/rulesis a rule, and the imported-rule case is the documented one. - Copilot's point on an unresolved link with a suffix hiding the warning does not apply: every link under
.claude/rulesthat does not resolve is named. - Copilot's point on a directory link named like a rule does not apply: an entry that resolves to a directory is named before it is classified.
- exai-bot's point on gating the
.claude/rulesexclusion on the profile flag does not apply: the exclusion and the_importsskip depend onrules, and a Codex-profile test covers it. - exai-bot's point on unindented flow-list continuation lines do not apply: they stay in the
pathsblock while a list is open. - exai-bot's point on brackets in YAML comments do not apply:
_flow_depthignores them, with two tests. - exai-bot's point on an alias of a rule being import-expanded does not apply:
_importsmatches the resolved target, and a test pins it. - exai-bot's point on an indented field after
paths: []does not apply: an indented mapping key ends the block, with parametrized cases. - exai-bot's point on a scoped rule hiding a cut-import warning does not apply: only a
startrule counts as loaded, and a test pins the scoped case. - exai-bot's point on quoted keys after
paths: []do not apply: double- and single-quoted keys end the block, with cases. - exai-bot's point on a scoped alias of AGENTS.md hiding a cut import does not apply: independently loaded guidance is snapshotted before rule promotion, and a test pins it.
- exai-bot's point on warning on a dangling non-Markdown link under
.claude/rulesdoes not apply: the maintainer chose to warn on any dangling link there, and a test pins it. - exai-bot's point on unscoped nested rules and cut-import warnings do not apply: an unscoped nested rule imported from its own subtree is quiet, with tests for the scoped and out-of-subtree siblings.
- exai-bot's point on import order hiding what a rule link leads to does not apply: expansion is tracked per route kind, and a test covers both orders.
- exai-bot's point on an ordinary import losing frontmatter that a rule link removes does not apply: the ordinary import replaces the rule entry with its full count.
- exai-bot's point on frontmatter stripping when the profile does not read rules does not apply: the flag reaches every
_importscall, and a rules-off test covers it. - exai-bot's point on a rule imported through a linked rules directory keeping its frontmatter does not apply: the 5-character body is counted, and a test pins it.
- exai-bot's point on an imported rule linked to off guidance keeping the full count does not apply: the body count is used, and a test pins it.
- exai-bot's point on
_importslosing its argument defaults does not apply:max_depth=None, rules=Falseare present at this head. - exai-bot's point on the docs omitting AGENTS.md import chains does not apply: the contract and the architecture page name AGENTS.md as well as CLAUDE.md.
- exai-bot's point on the receipt's
budget()call ignoring the rules flag does not apply:repo.pypassesprofile.reads_claude_rules, and a test covers it. - exai-bot's point on a root cut hidden by a nested CLAUDE.md does not apply: the exemption skips pairs the start chain cuts, and a test pins it.
- exai-bot's point on an explicit import through a linked rules directory keeping its frontmatter does not apply: the body is counted, and a test pins it.
- exai-bot's point on an ordinary import of a rule behind a linked rules directory does not apply: it replaces the body-only count in either order.
- exai-bot's point on the receipt dropping
ruleentries does not apply:_budget_whenincludesrule, and a test runs realbudget()output throughbuild_receipt(). - exai-bot's point on a cut import blamed on only the first root does not apply: provenance is kept per root, and a test pins it.
- exai-bot's point on root provenance past the hop limit does not apply: each root is walked within the limit, and a test pins it.
- exai-bot's point on gating rule-route tracking on
rulesdoes not apply: with rules off a rule-named link shares one reached set, and a test pins it. - exai-bot's point on a rule import through a linked rules directory keeping guidance frontmatter does not apply: the body count is used, and a test pins it.
- exai-bot's point on nested guidance reached by a rule import keeping its frontmatter does not apply: the rule's start time and body count win, and a test pins it.
- exai-bot's point on a scoped rule alias of nested AGENTS.md giving a wrong receipt does not apply: such a file is no longer expected in its subtree run, and a test pins it.
- exai-bot's point on merged provenance for two routes to one target does not apply: provenance is keyed by the path read, and a test pins it.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to ba59f29, which fix the nit from my review:
ba59f29:dokimi/tests/test_repo_budget_rules.py:157: added a rules-off case asserting an imported.claude/rules/r.mdwith frontmatter counts 26 chars.
make ci-local at ba59f29: 30 passed, 0 failed, 7 skipped.
GitHub CI has not run on these commits: the branch conflicts with main, and Actions runs no workflow on a conflicting PR. CI will check them once main is merged in.
Not approving yet. Still open: the earlier thread of mine above nit level.
A `.claude` link to a directory with rules/ loads those rules at start, but the tree lists it as one entry: they were counted lazy under the target's path or dropped, with no warning. Warn on such a link and on a `.claude` link that does not resolve in the tree. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Neither a work tree nor a commit lists the files of a submodule, so a .claude/rules submodule (a common way to share rules) dropped every rule from start with no warning. Warn for each .gitmodules path at .claude or at or below .claude/rules. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
The nested-rule exemption that silences a cut import applies only to a harness that reads rules. Nothing failed when that gate was made unconditional, so a rules-off --fail-on-warning run could pass quietly. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Conflicts resolved additively: budget() takes rules beside follow_imports and _followed_imports passes rules on; claude_rules sits beside shadow_file in profile, schema and yaml; the contract keeps both texts. Tests that assume an AGENTS.md turned off pass claude_displaces_agents=True (intel#379 gates the root rule); the receipt test profile gains follows_imports. repo.py keeps the 800-line count by joining one argument line. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
…tmatter.py The merge put budget.py at 807 lines, over the 800-line cap; the paths-frontmatter parser moves byte for byte (699 + 118 lines). budget imports the three names it uses; the stringify test patches yaml through the new module. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
|
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at e2f082a. The new warnings for a root .claude link and for a rules submodule are right, and the move of the frontmatter reader is a pure move. One risk from my earlier review is still open, and there is one nit inline.
What I ran, on a Linux review host at e2f082a:
make ci-local: 30 passed, 0 failed, 7 skipped (docker, network and org-level jobs only); 14,354 tests; changed-line coverage gate passes.- Reproductions for the finding below, with no model or network involved.
- Mutation check on
budget.pyandunlisted_rules.pyagainst the budget and receipt test files (about 30 mutants): theif rules else set()mutant from my thread is now killed; the one below survives.
Still open from my earlier review:
dokimi/tests/test_repo_budget_rules.py:516:_target(tree, target) == tin_cut_is_quiet(budget.py:96) replaced byTruestill passes all 303 tests intest_repo_budget_rules.py,test_repo_budget.py,test_repo_budget_nested.pyandtest_repo_receipt.py. Withsub/CLAUDE.md=@notes.md,sub/notes.md, an unscopedsub/.claude/rules/r.md,import_max_depth=0andrules=True, the head warnssub/CLAUDE.md: import of sub/notes.md is past the harness's 0-hop import limit, not followed; the mutant returns[], so any cut import in a subtree with an unscoped rule goes quiet and--fail-on-warningpasses over it. The tests added since pin therules=Falsemutant and the different-root and scoped cases, not this one. Add that tree, expecting the warning.
Checked, no issues
- Refactor: every line removed from
budget.pyin e2f082a is inrule_frontmatter.py;ruffandmypyare clean and the growth ratchet passes. - New warnings: the work-tree and git-tree sources give the same warnings for a root
.claudelink (to a directory, out of the tree, dangling) and for a submodule at.claudeor at or below.claude/rules; a real.claudedirectory, a link to a directory withoutrules/, andrules=Falsestay silent. - Schemas and docs:
rulein thekindenum and optionalclaude_ruleswiden in place; the contract, the architecture page and the changelog name the new warnings. - Commits: every non-merge commit carries a
Signed-off-by.
Bot points I checked that no longer apply at this head (41)
- exai-bot's points on the docs omitting an imported rule and on AGENTS.md import chains do not apply: the contract, the architecture page and the changelog state the imported-rule case and its
startclass, and name AGENTS.md as well as CLAUDE.md. - exai-bot's and Copilot's points on enforcing non-expansion of an imported rule's
@paths do not apply: the maintainer chose to follow them within the hop limit, the docs say so, and tests pin it. - exai-bot's points on a rule linked to a guidance file or to a nested rule losing startup context or keeping its frontmatter do not apply: the entry takes the earlier loading condition and the rule's body count, and tests pin each case.
- exai-bot's point on an oversized integer crashing the frontmatter parser does not apply:
ValueErroris caught and thepathslines are read. - exai-bot's points on the unparsed-YAML
pathsfallback (comments,null, empty items,true, split flow lists, brackets in comments, indented and quoted keys) do not apply: each case reads as the parsed YAML would, and parametrized cases pin them. - Copilot's and exai-bot's points on dangling links under
.claude/rulesand on a directory link named like a rule do not apply: every unresolved link warns (the maintainer chose that), and an entry that resolves to a directory is named before it is classified. - exai-bot's point on gating the
.claude/rulesexclusion on the profile flag does not apply: the exclusion and the_importsskip depend onrules, and a Codex-profile test covers it. - exai-bot's point on an alias of a rule being import-expanded does not apply:
_importsmatches the resolved target, and a test pins it. - exai-bot's points on a scoped rule or a scoped alias of AGENTS.md hiding a cut-import warning do not apply: only a
startrule counts as loaded, the independently loaded guidance is snapshotted before rule promotion, and tests pin both. - exai-bot's point on unscoped nested rules and cut-import warnings does not apply: an unscoped nested rule imported from its own subtree is quiet, with tests for the scoped and out-of-subtree siblings.
- exai-bot's point on import order hiding what a rule link leads to does not apply: expansion is tracked per route kind, and a test covers both orders.
- exai-bot's points on an ordinary import losing the frontmatter that a rule link removes, and on frontmatter stripping when the profile does not read rules, do not apply: the ordinary import replaces the rule entry with its full count in either order, and the flag reaches every
_importscall. - exai-bot's points on a rule imported through a linked rules directory keeping its frontmatter do not apply: the body is counted, including for rule-named guidance, and tests pin it.
- exai-bot's point on
_importslosing its argument defaults does not apply:max_depth=None, rules=Falseare present at this head. - exai-bot's point on the receipt's
budget()call ignoring the rules flag does not apply:repo.pypassesprofile.reads_claude_rules, and a test covers it. - exai-bot's point on a root cut hidden by a nested CLAUDE.md does not apply: the exemption skips pairs the start chain cuts, and a test pins it.
- exai-bot's point on the receipt dropping
ruleentries does not apply:_budget_whenincludesrule, and a test runs realbudget()output throughbuild_receipt(). - exai-bot's points on cut-import provenance (only the first root, roots past the hop limit, routes merged by target) do not apply: provenance is kept per root, bounded by the hop limit and keyed by the path read, and tests pin each.
- exai-bot's point on gating rule-route tracking on
rulesdoes not apply: with rules off a rule-named link shares one reached set, and a test pins it. - exai-bot's point on a scoped rule alias of nested AGENTS.md giving a wrong receipt does not apply: such a file is no longer expected in its subtree run, and a test pins it.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to 6385113, which fix the nit from my review:
6385113:dokimi/tests/test_repo_budget_rules.py:349: added a CRLF.gitmoduleswithpath=.claude/rules/and asserted the submodule warning.
make ci-local at 6385113: 30 passed, 0 failed, 7 skipped.
Not approving yet. Still open: the earlier thread of mine above nit level.
…ped rule A cut import in sub/ still warns when sub/ holds an unscoped rule and the rules flag is on. The exemption covers only the rule's own target, so a mutant that drops the target check passed every budget test before this. Thread: PRR_kwDOVBg7g88AAAABV9dH2Q Thread: PRR_kwDOVBg7g88AAAABV8BBLg Thread: PRR_kwDOVBg7g88AAAABVvL8CA Thread: PRR_kwDOVBg7g88AAAABV8ODBQ Thread: PRR_kwDOVBg7g88AAAABV-LK0Q Thread: PRR_kwDOVBg7g88AAAABVdu3zw Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Each item below is fixed by an earlier commit of this PR; the reply to each item names that commit. Thread: PRR_kwDOVBg7g88AAAABVUL41w Thread: PRR_kwDOVBg7g88AAAABVXCatA Thread: PRR_kwDOVBg7g88AAAABVXSpLg Thread: PRR_kwDOVBg7g88AAAABVYynbw Thread: PRR_kwDOVBg7g88AAAABVY6aYw Thread: PRR_kwDOVBg7g88AAAABVeDL4g Thread: PRR_kwDOVBg7g88AAAABVes3wA Thread: PRR_kwDOVBg7g88AAAABVir7vQ Thread: PRR_kwDOVBg7g88AAAABVswYFg Thread: IC_kwDOVBg7g88AAAABiyqZug Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. Fixed in 738c454; the reviewer confirmed the alias-chain fix there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. Each point is answered in its thread, and none reproduces at head: an unresolved link is named, a directory link is named and not counted as a rule, and a rule's @ paths are followed within the hop limit, as the contract says. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. Fixed in cd427e9 to 6d372ac (docs b1f2624); confirmed at 6d372ac. Docs nit fixed in b1f2624; the risks are fixed in cd427e9 to 6d372ac. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. Fixed in c0c911e; the reviewer confirmed it at c0c911e. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. Fixed in b484dc8, with both quoted cases pinned. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. Cut-warning branch pinned in 5570132; the other points are fixed. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. Fixed in 24bad8f, 89cd35e and ad57cb7; confirmed at ad57cb7. Fixed in 5fe9b15; the three points are in 24bad8f to 5fe9b15. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. Fixed in 5ade9e1 and cb27212; the reviewer agreed at cb27212. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. No claim in this review: the body is the bot's run marker. Its findings are the threads on this PR, and each has its answer there. Receipt points fixed in f96e654; main merged in a072eec. Open test gap pinned in 5570132; rules-off gate in ba59f29, c8d4a53. Mutant pinned by the sub/notes.md test in 5570132. Open test gap pinned in 5570132 (sub/notes.md tree). Test added in 5570132; the mutant that drops the target check fails it. CRLF case in 6385113; open cut-warning gap pinned in 5570132. Each item is fixed: be4c6da, 506fe5b, c8d4a53, 22ddcae, e2f082a. |
…intel#363 asked for Each new test fails on a one-line mutant that passed the whole suite; no code under dokimi/dokimi/ changes. intel#334 (context prefilter): - a write target glued to a heredoc sign (cat >notes.txt<<docs/guide.md) ends at the `<`: the delimiter is no path (_REDIRECT's `<` exclusion); - a bare-name rule `Makefile` counts an edit of Makefile only, not Makefile.bak or GNUMakefile, and counts C:\r\Makefile (_file_named); - `sudo echo changelog.d/x.md` prints the path, it does not act on it (strip_wrappers in _prints). intel#326 (repo budget): - an unclosed `paths: [null` or `paths: [~` reads as unscoped (_names_pattern's null and ~); - a rule reached through docs/alias.md drops its frontmatter before its @paths are followed (_import_text's _target half); - an import of a rule path linked to a plain file keeps the rule's body count (_counted_import's RULE_GLOB.match(f)). intel#363 (repo skill-reach): an OSError while saving the artifact exits 1 with a message; --bin reaches the harness run; --keep keeps the fixture and names it in the artifact. intel#323 (repo config): shared '**/Makefile' claims a root Makefile; a link c[1] on the way to the config is claimed as written; link_text keeps the `..` prefix of a subdirectory link that leaves the tree and comes back, and reads a sibling t2/ of root t as outside; a tracked `:x.sh` takes its mode from the index (--literal-pathspecs). Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
#389) Each new test fails on a one-line mutant that passed the whole suite; no code under dokimi/dokimi/ changes. #334 (context prefilter): - a write target glued to a heredoc sign (cat >notes.txt<<docs/guide.md) ends at the `<`: the delimiter is no path (_REDIRECT's `<` exclusion); - a bare-name rule `Makefile` counts an edit of Makefile only, not Makefile.bak or GNUMakefile, and counts C:\r\Makefile (_file_named); - `sudo echo changelog.d/x.md` prints the path, it does not act on it (strip_wrappers in _prints). #326 (repo budget): - an unclosed `paths: [null` or `paths: [~` reads as unscoped (_names_pattern's null and ~); - a rule reached through docs/alias.md drops its frontmatter before its @paths are followed (_import_text's _target half); - an import of a rule path linked to a plain file keeps the rule's body count (_counted_import's RULE_GLOB.match(f)). #363 (repo skill-reach): an OSError while saving the artifact exits 1 with a message; --bin reaches the harness run; --keep keeps the fixture and names it in the artifact. #323 (repo config): shared '**/Makefile' claims a root Makefile; a link c[1] on the way to the config is claimed as written; link_text keeps the `..` prefix of a subdirectory link that leaves the tree and comes back, and reads a sibling t2/ of root t as outside; a tracked `:x.sh` takes its mode from the index (--literal-pathspecs). Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Summary
repo budgetnow counts Claude Code.claude/rules/**/*.md. Before this, the docs admitted thatstartwas "too low by.claude/rules/*.md" for every repo that uses rules.guidance.claude_rulesis a new optional key.harness_profile.v1is widened in place,claude-code.yamlsets it totrue, and the Codex and Copilot profiles do not count rules.kindenum inrepo_budget.v1gainsrule.startrule. It is not also counted as an import.paths: [],"",nulland an empty key.paths:lines are still read, which matches what was measured. Frontmatter nested too deep for the parser falls back the same way and no longer crashes.Docs updated: the evaluation contract, architecture and what-it-measures. A changelog fragment is included.
Evidence
Before:
repo budgeton a repo with.claude/rules/plain.mdlisted no entry for it, sostartwas missing about 8k tokens.After: that file is reported as
kind: rule,when: start. The classification matches what Claude Code 2.1.304 actually loads:.claude/rules/plain.md.claude/rules/deep/x.mdpaths: ["src/**"]sub/.claude/rules/n.mdSee @extra.mdpathsLazy side: tokens on the second call, after a Read. The control is a scoped rule plus a non-matching Read, which adds +223.
paths: src/**, Readsrc/a.pysub/.claude/rules, Readsub/a.pypaths: [unclosed(invalid glob)paths: "src/**"+ broken YAML elsewherepaths: []Tests:
tests/test_repo_budget_rules.py(new, 20 cases). Eachpathsshape, each link warning and the too-deep frontmatter case failed before its fix and passes now. All 120 budget tests pass.Merge Danger
Door: two-way
Both schemas are widened in place: a new optional profile key and a new
kindvalue.Blast Radius: budget-consumers
repo budgettotals for claude-code go up on repos that have rules, so any stored baseline compared against them will show growth.kindvalues will now seerule.repo reachandrepo scanstill do not look at rules (follow-up).🤖 Generated with Claude Code