feat(core): broaden .understandignore starter for C#/Java/Go test patterns#466
feat(core): broaden .understandignore starter for C#/Java/Go test patterns#466thejesh23 wants to merge 3 commits into
Conversation
Detect C# project-suffix dirs (Foo.Tests/, Foo.UnitTests/) and PascalCase test dirs (Tests/, UnitTests/, IntegrationTests/) via case-insensitive match; group test-file suggestions by language (JS, C#, Java, Go). Keeps all suggestions commented-out — same opt-in model as today. Updates SKILL.md Phase 0.5 inline generator to stay in sync with the TS module. Refs Egonex-AI#76 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e-liner Replaces the duplicated Node.js block in Phase 0.5 with a call into `generateStarterIgnoreFile` via a thin wrapper script, mirroring the scan-project.mjs pattern. Removes ~40 lines of duplicated logic; single source of truth in @understand-anything/core. Also tightens code review nits: - Add 3 tests: stable language-group ordering, all-commented invariant on empty dirs, suffix-glob rejects non-directory entries - Clarify comments on EXACT_DIR_NAMES (ecosystem mix, not Python) and SUFFIX_DIR_GLOBS (unanchored String.endsWith match) - Type detectDirectories' readdirSync result explicitly (Dirent[]) to pin the utf-8 encoding overload Refs Egonex-AI#76 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0155c5b48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const __dirname = dirname(fileURLToPath(import.meta.url)); | ||
| // skills/understand/ -> plugin root is two dirs up | ||
| const pluginRoot = resolve(__dirname, '../..'); |
There was a problem hiding this comment.
Use the resolved plugin root for copied skill installs
When the skill directory is copied to a runtime skills folder instead of symlinked under the plugin checkout, resolve(__dirname, '../..') points at that skills parent rather than the real plugin root. Phase 0 already supports those installs by resolving PLUGIN_ROOT from several locations, but this new Phase 0.5 helper ignores that value, so first-time runs with no .understandignore fail to resolve @understand-anything/core and then try a nonexistent packages/core/dist/index.js under the skills folder. Use the resolved plugin root or implement the same fallback search here before importing core.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, addressed in 83d3fb6.
generate-ignore.mjs now prefers $PLUGIN_ROOT from the env (validated by checking for a package.json at that path) and falls back to the existing resolve(__dirname, '../..') heuristic. SKILL.md Phase 0.5 was updated to pass the env var:
PLUGIN_ROOT="$PLUGIN_ROOT" node <SKILL_DIR>/generate-ignore.mjs $PROJECT_ROOTSmoke-tested both paths locally — the env-set path resolves through Phase 0's multi-candidate value; the fallback path matches prior behavior for standard installs. 753 tests still pass; lint clean.
Note: the same resolve(__dirname, '../..') pattern exists in scan-project.mjs, compute-batches.mjs, extract-import-map.mjs, extract-structure.mjs, and build-fingerprints.mjs. They share the same latent issue but kept out of this PR's scope — happy to file a follow-up to harden them uniformly if the maintainers want.
…e.mjs Addresses codex review feedback (P2). `resolve(__dirname, '../..')` breaks when `skills/understand/` is copied to a runtime skills directory whose parent is not the plugin checkout — exactly the case SKILL.md Phase 0 warns about and resolves via its multi-candidate $PLUGIN_ROOT search. This script now prefers $PLUGIN_ROOT from the env (validated via package.json presence) and falls back to the existing relative resolution. SKILL.md Phase 0.5 passes the env var in the invocation. Same latent pattern exists in scan-project / compute-batches / extract-import-map / extract-structure / build-fingerprints; hardening those is a separate concern (no behaviour change today for installs that already work). Refs Egonex-AI#76 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Foo.Tests/,Foo.UnitTests/,Foo.IntegrationTests/) and PascalCase test dirs (Tests/,UnitTests/,IntegrationTests/) via case-insensitive match ingenerateStarterIgnoreFile.skills/understand/generate-ignore.mjsso Phase 0.5 invokes@understand-anything/coreinstead of duplicating ~40 lines of generator logic in an inlinenode -e "…"block.All suggestions remain commented-out — same opt-in model. No defaults change; no behaviour change for users with an existing
.understandignore.Motivation
.understandignorestarter today only suggests JS/TS/Python-shaped patterns: lowercase exact-name dirs (tests,__tests__,fixtures) and dot-separated test file patterns (*.test.*,*.spec.*). For C# / Java / Go projects this misses common conventions:Tests/(case-sensitive FS),Foo.Tests/,Foo.UnitTests/,*Tests.cs,*Test.cs,*.Tests.csprojsrc/test/,*Test.java,*IT.java,*Spec.kt*_test.goChanges
packages/core/src/ignore-generator.tsexistsSync(join(root, dir))probe withreaddirSync(root, { withFileTypes: true })+ case-insensitive exact-name match + suffix-glob match.EXACT_DIR_NAMES,SUFFIX_DIR_GLOBS,TEST_PATTERN_GROUPSconstants.Tests/nottests/when that's what they have.packages/core/src/__tests__/ignore-generator.test.tsskills/understand/generate-ignore.mjs(new)generateStarterIgnoreFilefrom@understand-anything/corevia the same workspace-link → plugin-cachedistresolution pattern used byscan-project.mjs.skills/understand/SKILL.mdnode -e "…"block to one line:node <SKILL_DIR>/generate-ignore.mjs $PROJECT_ROOT. Single source of truth for ignore generation now lives in@understand-anything/core.Out of scope (deliberate)
bin/inDEFAULT_IGNORE_PATTERNS— already flagged by @AsimRaza10 on How to make /understand run faster ? #76; leaving it for that thread..gitignoreruntime fallback for non-git projects — @AsimRaza10's PR.--excludeCLI flag — feat: add --exclude CLI flag to /understand for user-defined ignore p… #373.Test plan
pnpm --filter @understand-anything/core build— clean.pnpm --filter @understand-anything/core test— 753 tests pass (30 inignore-generator.test.ts, including 14 new).pnpm lint— clean.node skills/understand/generate-ignore.mjs /tmp/smokeagainst a directory withTests/,MyApp.Tests/,UnitTests/,fixtures/and a.gitignore. Output included# Tests/,# MyApp.Tests/,# UnitTests/,# fixtures/,.gitignoreextraction, and language-grouped patterns in stable order — all commented out./understandrun on a C# repo to confirm Phase 0.5 invokes the bundled script correctly.Refs #76
🤖 Generated with Claude Code