[antonwolfy]

Summary

Follow-up to the review of #323, which left two points open about how skill_sha and suite_hash hash a git work tree (review). The PR's own notes held the first one as an open question: (a) apply a nested repository's own ignore rules, one git ls-files per nested repository, or (b) treat the nested repository as opaque and document that. The author recommended (a), and this PR does (a).

1. Only the top work tree's ignored paths were read. A file that a nested repository or submodule inside a skill ignores still moved the hash.
2. A file whose name holds a newline, starts with " or ends with a CR hashed by its blob id while committed and unchanged, but by its raw bytes once edited or untracked. git hash-object --stdin-paths misreads such names, so they were kept away from it.

On main (51bbc4d), with a committed .github/dokimi/config.yaml (subject skill/a, path skills/a, suite skills/a/evals):

Case	main	This branch
skills/a/vendor is git inited with .gitignore *.pyc and a committed lib.py; then skills/a/vendor/lib.pyc is written (inner git status --porcelain is empty)	skill_sha sha256:0b004281ad124… → sha256:6bd35a793851e…	unchanged: sha256:0b004281ad124…
committed skills/a/we\nird.txt (one\n) edited to two\n, then committed	sha256:c2e5b88a6a51a… while edited, sha256:bfb3e4a63f74b… once committed	sha256:bfb3e4a63f74b… both times

Changes

• GitTree.skipped (dokimi/dokimi/utils.py) also checks each directory above a path for a .git (a directory, or a submodule's .git file). For each such nested repository it runs git -C <dir> ls-files --others --ignored --exclude-standard --directory -z once, caches the result on the tree (GitTree.nested), and leaves out what that listing names below the walk's base, as it already does for the top's ignored paths. A tree with no nested repository makes no extra git call. The nested .git is still left out. The nested repository's other files, tracked or untracked, are hashed as before: git hash-object run from the top, not the nested index. The top's listing moved into _ignored_paths, which both calls use.
• A nested repository's rules never leave out a path the top's index holds, or a directory above one: git stores those. _index_blobs returns that set (GitTree.tracked) from git ls-files -s, so it holds even when git status fails.
• A nested repository git cannot read (its ls-files fails: a broken .git file, a corrupt index, an owner git refuses) raises OSError naming its directory, and RepoConfig.subjects() reports it as ConfigError: cannot read <dir>: git cannot read this nested repository. The top's listing failing still gives no GitTree, and the bytes on disk are hashed.
• _blob_contents sends names that --stdin-paths misreads (a newline, a leading ", a trailing CR) to git hash-object -- <name>… as arguments, in batches under an argument-size budget (_ARG_BUDGET), so a large tree makes several calls. The other names still go on stdin. Such a file now hashes by its blob id whether it is committed or edited. The file_contents docstring no longer lists such a name as falling back to its bytes.
• Tests in tests/test_repo_config_git_blobs.py:
  • a file a nested repository ignores leaves the hash, and an edit to its tracked lib.py still moves it;
  • a file the top tracks under a nested repository's ignore rule (lib.pyc under *.pyc, build/out.txt under build/) still moves skill_sha when edited, and still does when git status fails, while an untracked build/new.txt stays out;
  • a nested repository with a corrupt index fails subjects() with that ConfigError, and a top repository whose ls-files fails hashes the bytes on disk;
  • three odd names (we\nird.txt, "quoted.txt, cr.txt\r, at the top of the work tree, where a top-relative name can start with ") digest the same edited and committed. Skipped on Windows, whose file names cannot hold these characters.
• Changelog fragment (Fixed).

Risk

skill_sha and suite_hash move, once, for a skill or suite in a git work tree that holds either:

• a file that a nested repository or submodule ignores, or
• an edited or untracked file with one of the odd names.

That is the intended effect. Trees without these do not change, and nothing changes outside a git work tree. A nested repository git cannot read, under a skill or suite, now fails subjects() with a ConfigError naming it; on main its files were hashed. The schema is unchanged. No doc describes the hash input apart from the #323 changelog fragment, so the docstrings and this PR's fragment carry the description.

Testing

On a Linux review host, at 8c375c6 (on main 5ce3207):

• make ci-local: 30 passed, 0 failed, 7 skipped. The skipped steps: external-catalog-smoke (needs docker), test-latest, docs deploy and pip-audit report (job conditions false on a PR), cli-surface's Package the result, conda-forge-deps (no micromamba), semgrep.
• 15688 tests passed, 10 skipped; changed-line coverage 100% (68 lines).
• Both repros above: they reproduce on main and are fixed here. A submodule (its .git is a file) behaves like the git inited directory: an ignored .pyc leaves skill_sha, and an edit to its tracked file moves it.
• The review's two cases reproduce at d1ca203 and are fixed here: an edit to a top-tracked s/v/lib.pyc or s/v/build/out.txt under a nested *.pyc / build/ rule left the digest unchanged; a nested repository with a corrupt index was hashed with nothing left out. A broken .git file and a refused owner (git's GIT_TEST_ASSUME_DIFFERENT_OWNER=1) now raise too.
• The new tests fail on main's code: 4 of 4 (the nested-repository test and all three names). Of this round's tests, the three top-tracked cases and the unreadable nested repository fail at d1ca203; the top-tracked cases pass on main, which never applied a nested repository's rules. The top-listing fallback test pins behaviour main already had.
• Each new test fails on the change it pins (mutants):
  • _inner_skips never consults the nested listing: the nested-repository test fails;
  • a .git holder is not detected: the nested-repository test fails;
  • odd names go through --stdin-paths too: all three name cases fail;
  • the trailing-CR check is dropped: cr.txt\r fails;
  • the leading-quote check is dropped: "quoted.txt fails;
  • skipped applies the nested rules to a top-tracked path: the three top-tracked cases fail;
  • GitTree.tracked holds files only, not the directories above them: the build/out.txt case fails;
  • GitTree.tracked is dropped when git status fails: the failed-status case fails;
  • an unreadable nested repository ignores nothing again: the corrupt-index test fails;
  • _ignored_paths returns an empty set when git fails: the corrupt-index and top-fallback tests fail.

Revert

A plain revert restores the old hashing. Hashes recorded under this change move back for trees that hold such files. No stored record or schema version is left behind.

Checklist

• Tests added/updated and pytest passes
• ruff, mypy, and schema checks pass
• Docs updated (or N/A) — see the docs-drift checklist in CONTRIBUTING.md
• Self-reviewed per "Before opening a PR" in AGENTS.md
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

[exai-bot]

✅ OpenCodeReview: Review complete: 0 finding(s) across 3 selected item(s).

CI at 8c375c6: 15 passing, 2 skipped.

Review details

• Commit reviewed: 8c375c6 against merge base 5ce3207
• Coverage: 3 files in 3 groups
• Engine: OpenCodeReview v1.13.5-exai.3.d7112371, us.openai.gpt-6.1-sol
• Usage: 180k tokens (100k from cache), 37s
• Tool calls: 9 (8 file_read, 1 file_read_diff)
• dokimi/changelog.d/20261104_120000_antonwolfy_tree-digest-nested-ignores.md: 1 file
• dokimi/dokimi/utils.py: 1 file
• dokimi/tests/test_repo_config_git_blobs.py: 1 file

[Copilot]

🟢 Approval recommended

There are no active review issues requiring changes.

No open issues 🎉

What changed in this PR

0 new findings; nothing to fix.

This follow-up to #323 makes skill_sha and suite_hash respect nested repositories’ ignore rules and consistently hash files with unusual names.

Changes:

• Cache and apply ignore rules from nested repositories and submodules.
• Pass filenames that Git’s stdin parser misreads as arguments, preserving blob-based hashing after edits.
• Add regression tests and document the affected hash changes.

File	Description
dokimi/​tests/​test_repo_config_git_blobs.py	Tests nested ignore rules and edited-versus-committed hashing for unusual filenames.
dokimi/​dokimi/​utils.py	Applies cached nested ignore rules and hashes unusual filenames through command-line arguments.
dokimi/​changelog.d/​20261104_120000_antonwolfy_tree-digest-nested-ignores.md	Documents both fixes and their effect on existing hashes.

_{Review effort: Balanced}

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.77%. Comparing base (5ce3207) to head (8c375c6).
⚠️ Report is 5 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #390      +/-   ##
==========================================
+ Coverage   94.75%   94.77%   +0.01%     
==========================================
  Files         352      352              
  Lines       46498    46455      -43     
  Branches     7969     7960       -9     
==========================================
- Hits        44061    44028      -33     
+ Misses       1544     1540       -4     
+ Partials      893      887       -6     

Files with missing lines	Coverage Δ
dokimi/dokimi/utils.py	93.86% <100.00%> (+1.72%)	⬆️

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[napetrov]

Verdict: no blocking thread and no inline thread; COMMENT review.

Description nit: the PR body says the odd names go to git hash-object "in one call", but _blob_contents batches them under _ARG_BUDGET (24 000 bytes), so a large tree makes several calls. The output is the same; reword that line.

Coverage: checked the diff, the nested-repository and odd-name paths in dokimi/dokimi/utils.py, and T1 (answered). Not looked at: the 10 digest test modules that fail collection here without regex (not installed, no network), so the test suite was not run; the batch-failure fallback (every odd-named file falls back to its raw sha256 if one batch fails) was not probed.

[antonwolfy]

@napetrov the description nit is done: the _blob_contents line now says the odd names go to git hash-object in batches under _ARG_BUDGET, so a large tree makes several calls. No code change.

[vlad-perevezentsev]

Review at d1ca203. The fix does what it says: a nested repository's ignored files leave skill_sha / suite_hash, and odd file names digest by blob id edited or committed. Two small edge cases are in the line comments.

Note: #388 (@napetrov) fixes the same nested-ignore problem in GitTree.skipped differently (it raises when git cannot read a nested repository) and conflicts with this PR in utils.py and test_repo_config_git_blobs.py. Only one of them should go in; please agree which.

What I ran, at d1ca203:

• make ci-local --offline: 0 failed, 19 passed, 13 skipped (network-only jobs).
• A repro for the top-tracked file under a nested .gitignore and for a nested repo with a broken gitfile (see the line comments).

Checked, no issues

• Argument batches: odd names go to git hash-object -- in batches under _ARG_BUDGET; the multi-batch and failed-batch tests cover the fix for the resolved bot thread.
• New tests: both fail on the base by the code (the inner .pyc was hashed; an edited odd name was hashed by raw bytes).
• Changelog: says the hash moves once for trees with such files.

[antonwolfy]

(commented) Review at d1ca203. The fix does what it says: a nested repository's ignored files leave `skill_

@vlad-perevezentsev (#390 (review)): Both edge cases from the line comments are fixed, each in its own commit. A top-tracked file under a nested repository's ignore rule counts again (9316f92). A nested repository git cannot read now raises a ConfigError naming it, rather than ignoring nothing (8c375c6). That is the behaviour the review describes for #388 on the unreadable case. This branch does not pick between the two PRs, and the conflict with #388 in utils.py and test_repo_config_git_blobs.py is still there. Which one goes in is for the maintainers and the author of #388 to settle.

[napetrov]

Approving at 8c375c6. Read the 2 new commits (top-tracked paths beat nested ignores; unreadable nested repo fails the hash), both tested. Looks right.

[antonwolfy]

Thanks for the review @napetrov. Nothing further to change from the approval; waiting for the remaining checks.