Skip to content

feat: add --exclude CLI flag to /understand for user-defined ignore p…#373

Open
yiziff wants to merge 2 commits into
Egonex-AI:mainfrom
yiziff:feat/add-exclude-flag
Open

feat: add --exclude CLI flag to /understand for user-defined ignore p…#373
yiziff wants to merge 2 commits into
Egonex-AI:mainfrom
yiziff:feat/add-exclude-flag

Conversation

@yiziff

@yiziff yiziff commented Jun 3, 2026

Copy link
Copy Markdown

…atterns

  • createIgnoreFilter() now accepts optional extraPatterns array (Layer 4, highest priority)
  • scan-project.mjs parses --exclude from CLI args and passes patterns through
  • project-scanner.md documents --exclude flag usage
  • SKILL.md wires --exclude into Phase 0/1 and updates argument-hint
  • 5 new test cases covering CLI priority, ! negation, and .understandignore interaction

Related: #76

Summary

Linked issue(s)

How I tested this

  • pnpm lint
  • pnpm --filter @understand-anything/core test
  • pnpm test
  • Manual smoke test (describe above)

Versioning

  • Version bumped in all five manifests, OR
  • N/A — internal/docs-only change

…atterns

- createIgnoreFilter() now accepts optional extraPatterns array (Layer 4, highest priority)
- scan-project.mjs parses --exclude from CLI args and passes patterns through
- project-scanner.md documents --exclude flag usage
- SKILL.md wires --exclude into Phase 0/1 and updates argument-hint
- 5 new test cases covering CLI priority, ! negation, and .understandignore interaction

Related: Egonex-AI#76
@yiziff

yiziff commented Jun 17, 2026

Copy link
Copy Markdown
Author

"Hi @Lum1104
Just checking in on this PR. I notice that other ongoing issues (like #466) have already mentioned this --exclude feature as part of the project's scalability roadmap.

Is there anything else I need to optimize, or any edge cases/tests I should add to get this merged? Thanks!"

@tirth8205

Copy link
Copy Markdown
Contributor

Reviewed the full diff, checked out the branch, and ran the tests.

Verdict: correct and well-scoped. The feature does what it claims.

What I verified:

  • createIgnoreFilter(projectRoot, extraPatterns = []) adds CLI patterns as Layer 4 after defaults and both .understandignore files, so they get highest priority and ! negation works in both directions. Backward compatible — existing single-arg callers are unaffected.
  • scan-project.mjs arg parsing is order-independent and defensive. I confirmed --exclude works before or after the positionals, and a trailing value-less --exclude is harmlessly ignored (the i + 1 < length guard prevents it from swallowing a positional).
  • Glob semantics are delegated to the ignore lib, so gitignore syntax is consistent with the rest of the system. End-to-end on a real repo, --exclude "tests/*,docs/*" dropped both dirs and reported filteredByIgnore=2; --exclude "!dist/" re-included a default-excluded dir.
  • Tests: all 22 ignore-filter tests pass, full core suite 675/675, lint clean. The 5 new cases are meaningful (priority over .understandignore, ! re-include, combination, empty-array no-op).

One thing worth a doc note (not blocking):

--exclude only takes effect on the full-scan path. It flows through Phase 1 (project-scannerscan-project.mjs) but is not persisted to config.json (unlike --language), and the incremental-update path (git diff --name-onlycompute-batches.mjs) never consults it. Practical consequence: on a first full run the excludes hold (excluded files never enter the graph, so later incremental updates ignore them anyway), but you cannot newly add --exclude on an incremental run to drop files already in the graph — that requires --full. A one-line note in the --exclude help text or SKILL.md ("newly-added excludes require --full to take effect") would set the right expectation.

Otherwise this is good to merge.

Addressed reviewer feedback: added a note in SKILL.md to clarify that newly added --exclude patterns require a --full scan to take effect.
@yiziff

yiziff commented Jun 22, 2026

Copy link
Copy Markdown
Author

Thanks for the thorough review and testing, @tirth8205 ! I completely agree with the incremental scan behavior. I've just pushed a commit to update SKILL.md with that exact warning about needing --full to apply new exclusions. Let me know if it looks good to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants