Repository navigation
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>
|
🔍 OpenCodeReview found 2 issue(s) in this PR. By severity: 2 medium
Review details
|
| > `.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.
| 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.
| 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.05% 94.03% -0.02%
==========================================
Files 297 297
Lines 38018 38128 +110
Branches 6424 6450 +26
==========================================
+ Hits 35756 35855 +99
- Misses 1452 1458 +6
- Partials 810 815 +5
🚀 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) |
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