Repository navigation
fix(repo): scan gitignored paths - #244
ndgrigorian wants to merge 9 commits into
Conversation
…solved (not failed) Guidance that warns about ignored files names paths the tree never Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
What changed in this PR
Adds .gitignore-aware handling to repo scanning so that missing paths which are intentionally ignored by the repository are treated as build outputs (unresolved) rather than renames (fail).
Changes:
- Introduces an L0
.gitignoreparser/matcher (GitIgnore) aligned withgitignore(5)semantics. - Updates L0 path resolution to downgrade missing-but-gitignored paths/directories/globs from
failtounresolved, including the ignoring rule in the “why”. - Adds tests validating matcher parity with
git check-ignoreand new scan outcomes for gitignored paths.
| File | Description |
|---|---|
| dokimi/dokimi/repos/l0/gitignore.py | New .gitignore parsing/matching implementation used by L0 scanning. |
| dokimi/dokimi/repos/l0/core.py | Integrates .gitignore checks into missing-path handling and caching. |
| dokimi/tests/test_repo_l0_gitignore.py | Adds parity tests against git check-ignore plus rule attribution tests. |
| dokimi/tests/test_repo_l0_cases.py | Adds cases asserting missing-but-ignored paths become unresolved and include rule provenance. |
| dokimi/docs/repo-evaluation-contract.md | Documents updated classification rules for missing gitignored paths. |
| dokimi/changelog.d/20261026_150000_ndgrigorian_scan-gitignored-paths.md | Changelog entry describing the behavioral change. |
| dokimi/tests/test_agent_profiles.py | Updates a string used by an API-name-guard test. |
Review effort: Lite
dpctl.tensor was removed previously, moved to dpnp Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
75ee694 to
9163ea3
Compare
antonwolfy
left a comment
There was a problem hiding this comment.
The matcher agrees with git check-ignore on the common forms (negation, nested files, **, anchoring, directory-only rules), but a legal .gitignore line can crash the scan, and two of the new unresolved paths depend on how the path is written. One bug, two risks and one nit, all inline.
What I ran, on Linux at 9163ea3:
make ci-local: 26 passed, 2 failed, 7 skipped; 8,591 tests; changed-line coverage 94%. The two failures are the gitleaks steps: the proxy returned 504 on the gitleaks download. Not a finding.- Reproductions for the findings through
scan_repo, with no network. - A differential run of
GitIgnore.ignored_byagainstgit check-ignore --no-indexover about 100 patterns (root and nested) and 41 paths. - 26 mutants of
gitignore.pyand_missing: 20 are killed.?crossing/, the#comment skip, the\#/\!escapes and trailing-space stripping survive the full suite.
Checked, no issues
- Git parity: no difference from git for
!under ignored directories,a/**,**/a/b,a/**/b,a**b,[]],[!a],[a-z], escaped trailing spaces, CRLF lines, and nested.gitignorefiles. - Unchanged cases: with no
.gitignorethe result staysfail; a path inside a submodule keeps its message; a tracked file under an ignored directory is stillok. - Growth baseline:
core.pystays under its cap andresolve_pathstays at its baseline complexity.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to 38e1a82, which fix the nit from my review:
38e1a82:dokimi/dokimi/repos/l0/gitignore.py:100: the rule now keepsline(only unescaped trailing spaces removed) instead ofraw.strip(), so the reason shows the rule as git read it.
make ci-local at 38e1a82: 26 passed, 2 failed (gitleaks download 504, host network), 7 skipped.
Not approving yet. Still open: the bug and the two risks from my review.
A bracket body was copied into a Python character class, so a tracked line such as `[z-a].h` raised re.error and ended repo scan. Brackets now follow wildmatch (POSIX classes, escapes, `]` first, a `/` inside), and a line wildmatch aborts on (unclosed `[`, trailing `\`) matches nothing. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Each `*` was an unbounded `[^/]*`, so a line like `*a*a*a*a*a*a*a*a*a*a*b` against a long missing name made repo scan hang. As in CPython's fnmatch, every star but the last now takes the fixed run after it atomically. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Every gitignore case scanned a root AGENTS.md, so dropping the guidance file's directory from the glob probe in _glob left all tests green while a nested `sub/*.cxx` turned from unresolved to fail. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
|
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 3e67c56. The bracket crash is fixed and the matcher now agrees with git check-ignore on the bracket forms from my earlier thread. Two risks from my earlier review are still open, and there is one nit inline.
What I ran, on Linux at 3e67c56:
make ci-local: 25 passed, 3 failed, 7 skipped; 8,596 tests; changed-line coverage passes. Two failures are the gitleaks install and the commit scan that needs it (the proxy returned 504 on the download); gitleaks run by hand over the PR's six commits found no leaks. The third istest_unbalanced_quote_in_hook_completes_quickly(1.7 s against a 1 s bound in the full run), which passes alone and failed only because other test suites ran on the machine at the same time. None of the three is a finding.- Reproductions through
scan_repofor the open threads. - A differential run of
GitIgnore.ignored_byagainstgit check-ignore --no-indexover about 10,800 generated patterns and 101 hand-picked cases, and 45 mutants ofgitignore.pyand_missing.
Still open from my earlier review:
dokimi/dokimi/repos/l0/core.py:309: a missing directory written without the trailing slash is stillfail. Guidance`pkg/dist`,pkg/.gitignore=dist/,pkg/a.cpresent givesfail("pkg/ exists but pkg/dist does not");`pkg/dist/`givesunresolved._missingstill tries only the file reading.dokimi/dokimi/repos/l0/core.py:331: onexinstance stands in for the whole glob. Stillfail:*.cxxwith!x.cxxand`pkg/*.cxx`;[0-9]*.pngwith`img/*.png`;gen.hwith`pkg/g*.h`;foo*.hwith!foox.hand`pkg/foo*.h`. The*.cxxcontrol without the!line givesunresolved, so the probe name decides the verdict.
I agree with Copilot that the x probe misclassifies globs under negation rules. Copilot's point on Rule.pattern does not apply: the rule now stores the line with only unescaped trailing spaces removed.
Checked, no issues
- Brackets:
[!]],[^]],[]],[z-a],[a\-z],[[:alpha:]]and the unclosed or unknown-class forms agree with git, and nothing raises; unusable lines match nothing. - Many stars:
*a*a*a*a*a*a*a*a*a*a*bagainst fiftyafinishes in milliseconds. - Unchanged cases: negation, anchoring, directory-only rules,
**and nested.gitignorefiles agree with git; the nested guidance file case is now pinned. - Repo rules: changelog fragment present, contract paragraph added, no generated artifacts.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to 62e8eb6, which fix the nit from my review:
62e8eb6:dokimi/tests/test_repo_l0_gitignore.py:41: added the five suggested lines toIGNORESand the eight suggested names toPATHS.
make ci-local at 62e8eb6: 26 passed, 2 failed (host), 7 skipped.
Not approving yet. Still open: the two earlier threads of mine above nit level; @copilot-pull-request-reviewer on the glob probe substitutes x for every wildcard, so *.cxx with !x.cxx is reported fail although the glob still names ignored files.
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at 62e8eb6. The matcher now reads .gitignore lines as git does, and a bad line no longer crashes the scan. What is left is in core.py: when a missing directory or glob counts as ignored. I agree with @antonwolfy's two open threads there (pkg/dist vs dist/, and the x probe for globs). The rest is in the line comments.
What I ran, at 62e8eb6:
make ci-local --offline: 16 passed, 1 failed, 13 skipped (8,543 tests passed). The failure isschema_compat:repo_manifest.v1.jsonandtask_inspect.v1.jsondiffer on main, and this PR does not touch any schema.GitIgnore.ignored_bychecked on[z-a].o,[!]]x,[a-]w,[[:digit:]]x, an unclosed[\and*a*a*a*a*a*a*a*a*a*a*bagainst fiftya: the answers match git, nothing raises, and the many-star line returns at once.- The parity test, run with
XDG_CONFIG_HOMEpointing at an ignore file that has*.csv.
Checked, no issues
- Brackets and stars: ad212fb reads classes as wildmatch does, and a line git cannot use matches nothing. The atomic groups in 793fa29 need Python 3.11, and that is the package's minimum.
- Rule text in the reason: it is the line with only unescaped trailing spaces removed, so Copilot's
raw.strip()point no longer applies. - Repo rules: the new files have the SPDX header, the contract paragraph is there, and no run artifacts are committed.
antonwolfy
left a comment
There was a problem hiding this comment.
One more nit, found by running this branch on a real repository: the reason names the .gitignore line only when the path's parent directory exists. Nothing has changed since 62e8eb6, so my earlier threads stand as they were.
What I ran, on Linux at 62e8eb6:
repo scanon IntelPython/dpnp at0cf6730(8AGENTS.mdfiles, 245 claims), on main (b69b15f) and on this branch: fails drop from 5 to 1.- Minimal trees through
scan_repofor the case inline. - The suggested change, with
tests/test_repo_l0_cases.pyandtests/test_repo_l0_gitignore.py(117 passed) andscripts/check_growth.py(clean).
Checked, no issues
- Real tree: the four claims that moved are Sphinx outputs,
doc/reference/generated/anddoc/reference/*.incin the root file and the same two written relative todoc/indoc/AGENTS.md. Each is nowunresolvedand names.gitignore:15or:16. The one fail left isTestXxx, a placeholder. No other claim changed status. - Links: a Markdown link to an ignored file (
[API](doc/_build/html/index.html)withdoc/_build/ignored) staysfail. That looks right to me: the link is broken where the guidance is read on GitHub, and the contract paragraph covers code-span paths only.
Not approving yet: my two threads above nit level are still open.
* Fix x probe to be more general * Fix for missing directories when lacking trailing forward slash Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
A glob was judged ignored by one made-up file, its wildcards filled in with `x`, so the verdict depended on that letter. `*.cxx` with `!x.cxx`, `[0-9]*.png`, `gen.h` and `gen_[a-c].h` all gave `fail` although git ignores real matches. The probe now fills the wildcards two ways (`x`, `dokimi-probe`) and adds each name an ignoring rule's last segment matches (`gen.h`, `0x.png`, `gen_a.h`). It keeps only the names the glob matches and asks the exact matcher about each. A glob written with a trailing slash is now checked as a directory. A missing directory was judged by the made-up name `<dir>/file`, which a whitelist (`*`, `!*/`, `!*.py`) always ignores, so a renamed directory read as `unresolved`. A directory now counts as ignored when git ignores it, or would ignore every name probed inside it: `file` and each name a `!` line re-includes. The reason now names the `.gitignore` line on the fallbacks too, where the parent directory is missing (`pkg/resources/cmake/`, `pkg/include/pkg/*.h`). It also no longer says the path is "not meant to be tracked", only that it is likely generated. The git oracle in the parity test sets HOME and XDG_CONFIG_HOME, so a developer's own core.excludesFile cannot change the answer. Both whitelist test cases now include `!.gitignore`; without it the `.gitignore` was never tracked and the cases checked nothing. Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
|
@xaleryb @antonwolfy @vlad-perevezentsev |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
5 open issues
resolve_path()builds multiple candidate paths (cands) across the file’s directory and its… · New This uses an atomic group ((?>...)) in the generated regex. Python’sreatomic grouping… · New This change (dpctl.tensor.empty()→dpnp.tensor.empty()) is unrelated to the PR’s stated… · New These tests hard-depend on agitexecutable being present at runtime. If the test environment can… · New Theassert < 1timing assertion is likely to be flaky on slower/contended CI runners and may… · New
3 resolved since last review
Review effort: Lite
| if hits: | ||
| return "ok_suffix", f"below a directory the text does not name; {len(hits)} match(es), e.g. {hits[0]}" | ||
| return "unresolved", "tried " + ", ".join(cands) | ||
| return self._missing([cands[0]], rel.endswith("/"), "tried " + ", ".join(cands), "unresolved") |
| def _git_ignored(root: Path) -> set[str]: | ||
| # HOME and XDG_CONFIG_HOME too: the default core.excludesFile is $XDG_CONFIG_HOME/git/ignore | ||
| env = {**os.environ, "GIT_CONFIG_GLOBAL": os.devnull, "GIT_CONFIG_NOSYSTEM": "1", | ||
| "HOME": str(root), "XDG_CONFIG_HOME": str(root)} | ||
| out = subprocess.run(["git", "-C", str(root), "check-ignore", "--no-index", "--", *PATHS], | ||
| capture_output=True, text=True, env=env, check=False) | ||
| assert out.returncode in (0, 1), out.stderr # 1: nothing ignored |
| start = time.perf_counter() | ||
| assert rules.ignored_by("pkg/" + "a" * 45) is None | ||
| assert rules.ignored_by("pkg/" + "a" * 45 + "b") is not None | ||
| assert time.perf_counter() - start < 1 |



Summary
This PR adds a feature to dokimi checking files provided in
AGENTS.mdfor presence in.gitignoreand ignores those files, as they are likely temporary or generated files likely not to be present in a given working tree.Changes
Risk
Should be backwards-compatible and low-risk, only works on edge cases (i.e., actively in-use repo).
Testing
Tested against dpctl repo
Revert
Remove: test_repo_l0_gitignore.py, gitignore.py, 20261026_150000_ndgrigorian_scan-gitignored-paths.md
revert in: core.py, test_repo_l0_cases.py, repo-evaluation-contact.md
Checklist
pytestpassesruff,mypy, and schema checks passAGENTS.md.github/.licenserc.yaml)