diff --git a/.claude/skills/bench-check/SKILL.md b/.claude/skills/bench-check/SKILL.md index a6f474d3..1b4145fa 100644 --- a/.claude/skills/bench-check/SKILL.md +++ b/.claude/skills/bench-check/SKILL.md @@ -153,6 +153,13 @@ For each metric in the current run: ## Phase 4 — Verdict +### Pre-condition check +Before evaluating verdicts, verify that at least one benchmark produced valid numeric results. +If `metrics` is empty (all suites produced `"error"` or `"timeout"` records): +- Print: `BENCH-CHECK ABORTED — no valid benchmark results (all suites failed or timed out)` +- Do NOT proceed to Phase 5 — the baseline must not be overwritten with empty data +- Stop here and skip to Phase 6 to write the report with verdict `ABORTED`. + Based on comparison results: ### No regressions found @@ -169,7 +176,7 @@ Based on comparison results: - Re-run individual benchmarks to confirm (not flaky) ### First run (no baseline) -- If `COMPARE_ONLY` is set: print a warning that no baseline exists and exit without saving +- If `COMPARE_ONLY` is set: print a warning that no baseline exists and **stop here — do not proceed to Phase 5 or Phase 6**. No baseline is saved and no report is written. - Otherwise: print `BENCH-CHECK — initial baseline saved` and save current results as baseline ### Save-baseline with existing baseline (`--save-baseline`) @@ -180,6 +187,7 @@ Based on comparison results: **Skip this phase if `COMPARE_ONLY` is set.** Compare-only mode never writes or commits baselines. **Skip this phase if regressions were detected in Phase 4.** The baseline is only updated on a clean run. +**Skip this phase if the ABORTED pre-condition was triggered in Phase 4.** The baseline must not be overwritten with empty data. When saving (initial run, `--save-baseline`, or passed comparison): @@ -211,8 +219,26 @@ git diff --cached --quiet -- generated/bench-check/baseline.json generated/bench ## Phase 6 — Report +**Skip this phase (write no report) if `COMPARE_ONLY` was set and no baseline existed, AND the ABORTED pre-condition was not triggered.** That early-exit case was already handled in Phase 4 — writing a "BASELINE SAVED" report here would be misleading since no baseline was saved. When ABORTED, always write the ABORTED report regardless of other flags. + Write a human-readable report to `generated/bench-check/BENCH_REPORT_.md`. +**If the ABORTED pre-condition was triggered (no valid benchmark results):** write a minimal report — this check MUST come before the SAVE_ONLY/first-run check, because when all benchmarks fail on a `--save-baseline` or first run, SAVE_ONLY would also be true but no baseline was actually saved: + +```markdown +# Benchmark Report — + +**Version:** X.Y.Z | **Git ref:** abc1234 | **Threshold:** $THRESHOLD% + +## Verdict: ABORTED — no valid benchmark results + +All benchmark suites failed or timed out. See Phase 1 error records for details. + +## Raw Results + + +``` + **If `SAVE_ONLY` is set or no prior baseline existed (first run):** write a shortened report — omit the "Comparison vs Baseline" and "Regressions" sections since no comparison was performed: ```markdown @@ -257,7 +283,7 @@ Write a human-readable report to `generated/bench-check/BENCH_REPORT_.md`. 1. If report was written, print its path 2. If baseline was updated, print confirmation -3. Print one-line summary: `PASSED (0 regressions) | FAILED (N regressions) | BASELINE SAVED` +3. Print one-line summary: `PASSED (0 regressions) | FAILED (N regressions) | BASELINE SAVED | ABORTED (all suites failed)` ## Rules @@ -266,6 +292,6 @@ Write a human-readable report to `generated/bench-check/BENCH_REPORT_.md`. - **Don't update baseline on regression** — the user must investigate first - **Recall/quality metrics are inverted** — a decrease is a regression - **Count metrics are informational** — graph growing isn't a regression -- **The baseline file is committed to git** — it's a shared reference point; Phase 5 always commits it +- **The baseline file is committed to git** — it's a shared reference point; Phase 5 commits it on clean (non-regressed) runs where COMPARE_ONLY is not set - **history.ndjson is append-only** — never truncate or rewrite it - Generated files go in `generated/bench-check/` — create the directory if needed diff --git a/.claude/skills/deps-audit/SKILL.md b/.claude/skills/deps-audit/SKILL.md index 083a8c77..8bf4de02 100644 --- a/.claude/skills/deps-audit/SKILL.md +++ b/.claude/skills/deps-audit/SKILL.md @@ -22,14 +22,10 @@ Audit the project's dependency tree for security vulnerabilities, outdated packa 5. **If `AUTO_FIX` is set:** Save the original manifests now, before any npm commands run, so pre-existing unstaged changes are preserved: ```bash git stash push -m "deps-audit-backup" -- package.json package-lock.json - STASH_CREATED=$? - ``` - Track `STASH_CREATED` — when `0`, a stash entry was actually created; when `1`, the files had no changes so nothing was stashed. - If `STASH_CREATED` is `0`, immediately capture the stash ref for later use: - ```bash STASH_REF=$(git stash list --format='%gd %s' | grep 'deps-audit-backup' | head -1 | awk '{print $1}') ``` - Use `$STASH_REF` (not `stash@{0}`) in all later stash drop/pop commands to avoid targeting the wrong entry if other stashes are pushed in the interim. + `STASH_REF` is non-empty if and only if a stash entry was actually created. Do **not** use `$?` — modern git (2.16+) returns 0 even when nothing was stashed. + Use `[ -n "$STASH_REF" ]` (stash created) / `[ -z "$STASH_REF" ]` (nothing stashed) for all branching. Use `$STASH_REF` (not `stash@{0}`) in all later stash drop/pop commands to avoid targeting the wrong entry. ## Phase 1 — Security Vulnerabilities @@ -165,13 +161,24 @@ If `AUTO_FIX` was set: Summarize all changes made: 1. List each package updated/fixed 2. Run `npm test` to verify nothing broke -3. If tests pass and `STASH_CREATED` is `0`: drop the saved state (`git stash drop $STASH_REF`) — the npm changes are good, no rollback needed - If tests pass and `STASH_CREATED` is `1`: no action needed — the npm changes are good and no stash entry exists to clean up -4. If tests fail and `STASH_CREATED` is `0`: +3. If tests pass and `STASH_REF` is non-empty: pop and merge the saved state (`git stash pop $STASH_REF`) — this restores any pre-existing uncommitted changes alongside the npm fix results. + - If the pop applies cleanly: + a. Run `npm install` to re-sync `node_modules/` with the merged manifest. + b. Re-run `npm test` to confirm nothing broke with the merged dependency state. + c. If tests still pass: confirm the project is consistent. + d. If tests now fail: warn the user — the pre-existing manifest changes conflict with the audit fixes. + Recovery options: + - To undo **all** manifest changes (both audit fixes and pre-existing): `git checkout -- package.json package-lock.json && npm ci` + - To keep only the audit fixes and discard pre-existing changes: manually edit `package.json`/`package-lock.json` to remove the pre-existing delta, then `npm ci` + - To keep only the pre-existing changes and discard the audit fixes: re-run `/deps-audit` without `--fix` + - If the pop causes conflicts in `package.json`/`package-lock.json`: warn the user, leave conflict markers for manual resolution, and instruct: "After you resolve the conflicts, run `npm install` to re-sync `node_modules/` with the resolved lock file before committing." + - For conflicts in other files, resolve them by keeping both the npm fixes and the pre-existing changes. + If tests pass and `STASH_REF` is empty: no action needed — the npm changes are good and no stash entry exists to clean up +4. If tests fail and `STASH_REF` is non-empty: - Restore the saved manifests: `git stash pop $STASH_REF` - Restore `node_modules/` to match the reverted lock file: `npm ci` - Report what failed -5. If tests fail and `STASH_CREATED` is `1`: +5. If tests fail and `STASH_REF` is empty: - Discard manifest changes: `git checkout -- package.json package-lock.json` - Restore `node_modules/` to match the reverted lock file: `npm ci` - Report what failed @@ -181,6 +188,6 @@ Summarize all changes made: - **Never run `npm audit fix --force`** — breaking changes need human review - **Never remove a dependency** without asking the user, even if it appears unused — flag it in the report instead - **Always run tests** after any auto-fix changes -- **If `--fix` causes test failures**, restore manifests from the saved state (`git stash pop $STASH_REF` if `STASH_CREATED=0`, or `git checkout` if stash was a no-op) then run `npm ci` to resync `node_modules/`, and report the failure +- **If `--fix` causes test failures**, restore manifests from the saved state (`git stash pop $STASH_REF` if `STASH_REF` is non-empty, or `git checkout` if nothing was stashed) then run `npm ci` to resync `node_modules/`, and report the failure - Treat `optionalDependencies` separately — they're expected to fail on some platforms - The report goes in `generated/deps-audit/` — create the directory if it doesn't exist diff --git a/.claude/skills/housekeep/SKILL.md b/.claude/skills/housekeep/SKILL.md index ef15efb7..7e10eb1f 100644 --- a/.claude/skills/housekeep/SKILL.md +++ b/.claude/skills/housekeep/SKILL.md @@ -66,28 +66,61 @@ For stale worktrees with merged branches: ## Phase 2 — Delete Dirt Files -Remove temporary and generated files that accumulate over time: +Remove temporary and generated files that accumulate over time. There are two distinct categories of dirt that require different discovery commands: + +- **Gitignored dirt** (files matching `.gitignore` patterns — e.g. `coverage/`, `.DS_Store`, `*.log`, `.codegraph/graph.db-journal`): use `git clean -fdX --dry-run` to list them. `git ls-files --others --exclude-standard` silently omits these because `--exclude-standard` suppresses gitignored entries. +- **Untracked non-ignored files** (stray files not in `.gitignore` — e.g. `*.tmp.*`, `*.bak`, `*.orig`): use `git ls-files --others --exclude-standard` to list them. + +Run both commands and union the results to get the full set of candidate dirt files. ### 2a. Known dirt patterns -Search for and remove: +Search for and remove files found by the two discovery commands above (never touch tracked files): - `*.tmp.*`, `*.bak`, `*.orig` files in the repo (but NOT in `node_modules/`) - `.DS_Store` files - `*.log` files in repo root (not in `node_modules/`) - Empty directories (except `.codegraph/`, `.claude/`, `node_modules/`) - `coverage/` directory (regenerated by `npm run test:coverage`) - `.codegraph/graph.db-journal` (SQLite WAL leftovers) -- Stale lock files: `.codegraph/*.lock` older than 1 hour + +**Stale lock files** (`.codegraph/*.lock` older than 1 hour): Before removing, check whether any process still holds the file. On Linux/macOS use `lsof "$f"` to verify. If the check is unavailable or the file is held, **require user confirmation** instead of deleting automatically — concurrent Claude Code sessions may hold legitimate long-lived locks. + +```bash +for f in .codegraph/*.lock; do + [ -f "$f" ] || continue + age=$(( $(date +%s) - $(stat --format='%Y' "$f" 2>/dev/null || stat -f '%m' "$f" 2>/dev/null) )) + [ -z "$age" ] && continue + if [ "$age" -gt 3600 ] && ! lsof "$f" > /dev/null 2>&1; then + if [ "$DRY_RUN" = "true" ]; then + echo "[DRY RUN] Would remove stale lock: $f" + else + echo "Removing stale lock: $f" + rm "$f" + fi + elif [ "$age" -gt 3600 ]; then + echo "Lock file $f is old but still held by a process — ask user before removing" + fi +done +``` ### 2b. Large untracked files -Find untracked files larger than 1MB: +Find untracked files (both gitignored and non-ignored) larger than 1MB. Use both discovery commands and union the paths: ```bash +# Non-ignored untracked files git ls-files --others --exclude-standard | while read f; do size=$(stat --format='%s' "$f" 2>/dev/null || stat -f '%z' "$f" 2>/dev/null) [ -z "$size" ] && continue if [ "$size" -gt 1048576 ]; then echo "$f ($size bytes)"; fi done +# Gitignored files (strip the leading "Would remove " prefix from dry-run output) +git clean -fdX --dry-run | sed 's/^Would remove //' | while read f; do + # Skip directory entries — stat returns inode size, not content size + [ -d "$f" ] && continue + size=$(stat --format='%s' "$f" 2>/dev/null || stat -f '%z' "$f" 2>/dev/null) + [ -z "$size" ] && continue + if [ "$size" -gt 1048576 ]; then echo "$f ($size bytes) [gitignored]"; fi +done ``` Flag these for user review — they might be accidentally untracked binaries. @@ -117,7 +150,7 @@ git log HEAD..origin/main --oneline ``` If main has new commits: -- If on main: `git pull origin main` +- If on main: `git pull --no-rebase origin main` - If on a feature branch: inform the user how many commits behind main they are - Suggest: `git merge origin/main` (never rebase — per project rules) diff --git a/.claude/skills/test-health/SKILL.md b/.claude/skills/test-health/SKILL.md index 4c836586..210e949c 100644 --- a/.claude/skills/test-health/SKILL.md +++ b/.claude/skills/test-health/SKILL.md @@ -41,8 +41,14 @@ Run the full test suite `FLAKY_RUNS` times and track per-test pass/fail: RUN_DIR=$(mktemp -d /tmp/test-health-XXXXXX) for i in $(seq 1 $FLAKY_RUNS); do timeout 180 npx vitest run --reporter=json > "$RUN_DIR/run-$i.json" 2>"$RUN_DIR/run-$i.err" - if [ $? -eq 124 ]; then + exit_code=$? + if [ $exit_code -eq 124 ]; then echo '{"timeout":true}' > "$RUN_DIR/run-$i.json" + elif [ $exit_code -ne 0 ] && [ $exit_code -ne 1 ]; then + # Use jq to safely JSON-escape stderr content (may contain quotes, newlines, backslashes) + stderr_content=$(cat "$RUN_DIR/run-$i.err") + jq -n --argjson code "$exit_code" --arg stderr "$stderr_content" \ + '{"error":true,"exit_code":$code,"stderr":$stderr}' > "$RUN_DIR/run-$i.json" fi done ``` @@ -139,7 +145,7 @@ Find files with < 50% line coverage. For each: Compare against `main` branch to find recently changed files: ```bash -git diff --name-only main...HEAD -- src/ +git diff --name-only origin/main...HEAD -- src/ ``` For each changed source file, check if: diff --git a/docs/roadmap/ROADMAP.md b/docs/roadmap/ROADMAP.md index 4614c75e..538b6e1b 100644 --- a/docs/roadmap/ROADMAP.md +++ b/docs/roadmap/ROADMAP.md @@ -1086,7 +1086,7 @@ npm workspaces (`package.json` `workspaces`), `pnpm-workspace.yaml`, and `lerna. **Why after Phase 4:** The resolution accuracy work (Phase 4) operates on the existing JS codebase and produces immediate accuracy gains. TypeScript migration builds on Phase 3's clean module boundaries to add type safety across the entire codebase. Every subsequent phase benefits from types: MCP schema auto-generation, API contracts, refactoring safety. The Phase 4 resolution improvements (receiver tracking, interface edges) establish the resolution model that TypeScript types will formalize. -**Note:** `.js` and `.ts` coexist during migration (`allowJs: true` in tsconfig). PRs #553, #554, #555, #566 migrated a first wave of files across steps 5.3–5.5, but substantial work remains in each step. 13 stale `.js` files have `.ts` counterparts and need deletion. +**Note:** `.js` and `.ts` coexist during migration (`allowJs: true` in tsconfig). PRs #553, #554, #555, #566 migrated a first wave of files across steps 5.3–5.5. Step 5.3 is complete; substantial work remains in steps 5.4–5.5. 13 stale `.js` files have `.ts` counterparts and need deletion. ### ~~5.1 -- Project Setup~~ ✅ @@ -1110,42 +1110,28 @@ Comprehensive TypeScript type definitions for the entire domain model — symbol **New file:** `src/types.ts` ([#516](https://github.com/optave/codegraph/pull/516)) -### 5.3 -- Leaf Module Migration (In Progress) +### ~~5.3 -- Leaf Module Migration~~ ✅ -Migrate modules with no or minimal internal dependencies. 25 migrated, 4 remaining. +Migrate modules with no or minimal internal dependencies. All 29 modules migrated. -**Migrated (25):** `shared/errors`, `shared/kinds`, `shared/normalize`, `shared/paginate`, `shared/constants`, `shared/file-utils`, `shared/generators`, `shared/hierarchy`, `infrastructure/logger`, `infrastructure/config`, `infrastructure/native`, `infrastructure/registry`, `infrastructure/update-check`, `infrastructure/result-formatter`, `infrastructure/test-filter`, `db/repository/*` (14 files), `domain/analysis/*` (9 files), `presentation/colors`, `presentation/table` — via [#553](https://github.com/optave/codegraph/pull/553), [#566](https://github.com/optave/codegraph/pull/566) - -**Remaining (4):** - -| Module | Notes | -|--------|-------| -| `src/db/connection.js` | SQLite connection wrapper | -| `src/db/index.js` | DB barrel/schema entry point | -| `src/db/migrations.js` | Schema version management | -| `src/db/query-builder.js` | Dynamic query builder | +**Migrated (29):** `shared/errors`, `shared/kinds`, `shared/normalize`, `shared/paginate`, `shared/constants`, `shared/file-utils`, `shared/generators`, `shared/hierarchy`, `infrastructure/logger`, `infrastructure/config`, `infrastructure/native`, `infrastructure/registry`, `infrastructure/update-check`, `infrastructure/result-formatter`, `infrastructure/test-filter`, `db/repository/*` (14 files), `db/connection`, `db/index`, `db/migrations`, `db/query-builder`, `domain/analysis/*` (9 files), `presentation/colors`, `presentation/table` — via [#553](https://github.com/optave/codegraph/pull/553), [#566](https://github.com/optave/codegraph/pull/566) ### 5.4 -- Core Module Migration (In Progress) -Migrate modules that implement domain logic and Phase 3 interfaces. Some migrated via [#554](https://github.com/optave/codegraph/pull/554), 39 files remaining. +Migrate modules that implement domain logic and Phase 3 interfaces. Some migrated via [#554](https://github.com/optave/codegraph/pull/554), 22 files remaining. -**Migrated:** `db/repository/*.ts` (14 files), `domain/parser.ts`, `domain/graph/resolve.ts`, `extractors/*.ts` (11 files), `domain/graph/builder.ts` + `context.ts` + `helpers.ts` + `pipeline.ts`, `domain/graph/watcher.ts`, `domain/search/{generator,index,models}.ts`, `graph/model.ts`, `graph/algorithms/{bfs,centrality,shortest-path,tarjan}.ts`, `graph/algorithms/leiden/rng.ts`, `graph/classifiers/{risk,roles}.ts` +**Migrated:** `db/repository/*.ts` (14 files), `domain/parser.ts`, `domain/graph/resolve.ts`, `extractors/*.ts` (11 files), `domain/graph/builder.ts` + `context.ts` + `helpers.ts` + `pipeline.ts`, `domain/graph/watcher.ts`, `domain/search/{generator,index,models}.ts`, `graph/model.ts`, `graph/algorithms/{bfs,centrality,shortest-path,tarjan}.ts`, `graph/algorithms/leiden/*.ts` (7 files), `graph/algorithms/{louvain,index}.ts`, `graph/builders/{dependency,structure,temporal,index}.ts`, `graph/classifiers/{risk,roles,index}.ts`, `graph/index.ts`, `domain/queries.ts` -**Remaining (39):** +**Remaining (22):** | Module | Files | Notes | |--------|-------|-------| | `domain/graph/builder/stages/` | 9 | All 9 build pipeline stages (collect-files, parse-files, resolve-imports, build-edges, etc.) | | `domain/graph/builder/incremental.js` | 1 | Incremental rebuild logic | | `domain/graph/{cycles,journal,change-journal}.js` | 3 | Graph utilities | -| `domain/queries.js` | 1 | Core query functions | | `domain/search/search/` | 6 | Search subsystem (hybrid, semantic, keyword, filters, cli-formatter, prepare) | | `domain/search/stores/` | 2 | FTS5, SQLite blob stores | | `domain/search/strategies/` | 3 | Source, structured, text-utils strategies | -| `graph/algorithms/leiden/` | 6 | Leiden community detection (adapter, CPM, modularity, optimiser, partition, index) | -| `graph/algorithms/{louvain,index}.js` | 2 | Louvain + algorithms barrel | -| `graph/builders/` | 4 | Dependency, structure, temporal builders + barrel | -| `graph/classifiers/index.js` + `graph/index.js` | 2 | Barrel exports | ### 5.5 -- Orchestration & Public API Migration (In Progress)