diag(extraction): log when the git-based scan falls back to a filesystem walk - #1568
Draft
milos85vasic wants to merge 1 commit into
Draft
Conversation
…tem walk getGitVisibleFiles() silently returned null on ANY exception (git missing, a rev-parse/ls-files timeout or buffer overrun under load, an unreadable repo, etc.), sending every caller to the independent scanDirectoryWalk() filesystem-walk fallback with zero signal that the fast, fully git-delegated path was skipped. That makes a report like "codegraph walked into a directory a nested (non- root) .gitignore excludes for git" nearly untriageable after the fact: both scanning implementations exist to respect .gitignore correctly on their own reading, but there is currently no way to tell which of the two actually ran for a given index/init/sync — so a maintainer investigating such a report has to re-instrument the source themselves to even confirm the fallback fired. This adds one logDebug() call (the project's existing CODEGRAPH_DEBUG-gated convention, already used elsewhere in this same file) naming the exception before falling back, with zero behavior change otherwise. Context: reported against v1.5.0 in a real ~63x file-count blowup (509 -> 32,260 files) on a project with git-confirmed-correct nested .gitignore exclusions (git check-ignore -v: frontend/.gitignore:10 and extension/.gitignore:2 both correctly exclude their node_modules trees). Neither scanning path in the current source could be shown to mis-handle a synthetic nested-.gitignore fixture in isolation (tested up to 63 nested .gitignore files / 960 excluded files / .bin symlinks against the actual v1.5.0 binary), so this defensive-hardening diagnostic is offered instead of a speculative behavioral fix for the reported regression itself. See the linked issue for full reproduction evidence.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds one
logDebug()call ingetGitVisibleFiles()'s outercatchblock, naming the exception before falling back to the independentscanDirectoryWalk()filesystem-walk path. Zero behavior change — purely diagnostic, gated by the project's existingCODEGRAPH_DEBUGconvention (same as every otherlogDebug()call in this file).Why
getGitVisibleFiles()currently silently returnsnullon any exception (git missing, arev-parse/ls-filestimeout or buffer overrun under load, an unreadable repo, etc.), sending every caller toscanDirectoryWalk()with no signal that the fast, fully git-delegated path was skipped.That makes a report like #1567 (a real ~63x file-count blowup where
git check-ignore -vproves git itself correctly excludes a nested-.gitignore'dnode_modulestree, yetcodegraph initwalked into it anyway) hard to triage after the fact: bothgetGitVisibleFiles()andscanDirectoryWalk()independently implement nested-.gitignorehandling and both look correct to me on inspection, but there is currently no way to tell which one actually ran for a givenindex/init/syncon the reporting host. This PR closes that specific observability gap.What this is NOT
This is not a fix for #1567's regression itself. I traced both scanning implementations and could not identify a concrete defect in either through static reading, and I could not reproduce the reported blowup in two synthetic fixtures (up to 63 nested
.gitignorefiles / 960 excluded files /.binsymlinks) run against the actualv1.5.0binary — see the issue for full detail on both the confirmed root-cause evidence and the inconclusive reproduction attempts. I'm offering this small, safe, unambiguously-correct diagnostic improvement now rather than guessing at a behavioral fix I can't prove addresses the real trigger.Testing
npx tsc --noEmit -p .— clean, no new errors.catchbranch gains a log call using the existinglogDebug(message, context)signature already used elsewhere in this same file (e.g.'Skipping unresolvable directory').Related