fix: audit findings — 3 HIGH bugs, perf, docs, cleanup

[SutuSebastian]

Summary

Addresses all actionable findings from cross-audit triangulation of three independent AI audits (Composer-2-fast, GPT-5.4-extra-high, Opus-4.6-max).

Bug fixes (3 HIGH, reproduced)

• Bun Worker onerror — parse worker crash silently hangs the full-rebuild pipeline; added onerror handler + worker.terminate()
• Component false-positives — PascalCase functions in .tsx unconditionally indexed as components; now requires JSX return or hook usage (5 new test cases)
• Incremental/--files skips custom extensions — files indexed during --full via custom include patterns silently go stale; now checks files table for previously-indexed paths

Performance (4 SQLite improvements)

• Batch CSS imports instead of one-at-a-time (insertImports called once per file, not per import)
• Statement cache (Map<string, Statement>) for better-sqlite3 run()/query() — eliminates ~2,000+ redundant prepare() calls
• Hoist inner.query() in wrap() — prepare once, not per .get()/.all()
• Skip PRAGMA optimize on closeDb for read-only query paths — avoids SQLITE_BUSY contention

Docs

• Wyhash → SHA-256 (architecture.md, SKILL.md x2)
• symbols.kind: variable → const, type_alias → type; exports.kind corrected
• Database.query() caching claim clarified as Bun-only; Node statement cache documented
• architecture.md: component heuristic, statement cache, closeDb readonly, incremental/--files custom extensions
• benchmark.md and golden-queries.md updated for enriched fixture

Testing

• Enrich fixtures/minimal/ to cover all 10 indexed tables (CSS module with @keyframes + @import, non-component PascalCase export, FIXME marker, second CSS variable, class selector)
• 7 new golden scenarios: exports, css_variables, css_classes, css_keyframes, css_imports, markers-all-kinds, components-no-false-positives
• All 13 Tier A scenarios pass

Cleanup

• Remove unused analyzeDependencies: true from CSS parser
• Deduplicate fetchTableStats across index-engine.ts / run-index.ts
• Remove 4 dead eslint-disable-next-line directives (oxlint doesn't enforce those rules)
• Fix SCHEMA_VERSION comment mismatch

Test plan

• All 108 non-sandbox tests pass (bun test)
• All 13 golden scenarios pass (bun run test:golden)
• Full rebuild smoke test (bun src/index.ts --full)
• Query path verified (bun src/index.ts query --json "SELECT ...")
• Component false-positive reproduction confirmed fixed
• Format, lint, typecheck all green
• Pre-commit hooks pass on every commit

Summary by CodeRabbit

• Bug Fixes

  • Prevented silent worker hangs on parse crashes
  • Improved component detection to avoid .tsx false positives
  • Ensured incremental and --files indexing include previously indexed custom-extension files

• Performance

  • Batched CSS import processing
  • Reduced redundant DB statement preparation with caching
  • Skip costly DB optimizations for read-only queries

• Documentation

  • Clarified hashing uses SHA-256 and updated symbol/export kind descriptions

• Tests

  • Added/expanded golden scenarios and component detection tests

[changeset-bot]

🦋 Changeset detected

Latest commit: 9b323fd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@stainless-code/codemap	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR fixes incremental indexing for custom extensions, refines component detection to require JSX or hooks, adds Bun worker onerror handling, batches CSS import inserts, caches prepared SQLite statements, skips PRAGMA on readonly closes, and updates docs/schema hashing to SHA-256.

Changes

Cohort / File(s)	Summary
Documentation & Schema /.agents/skills/codemap/SKILL.md, templates/agents/skills/codemap/SKILL.md, docs/architecture.md, docs/benchmark.md, docs/golden-queries.md	Updated docs and schema text: files.content_hash now described as SHA-256 hex digest; symbols.kind/exports.kind enumerations and component heuristic docs clarified; incremental indexing and readonly DB behavior documented.
Changelog / Changeset .changeset/audit-findings.md	Added changeset recording three cross-audit high-severity fixes and multiple performance/documentation/test updates.
Indexing & CLI src/application/index-engine.ts, src/application/run-index.ts, src/cli/cmd-index.ts	Expanded incremental include predicate to keep previously indexed paths regardless of extension; --files now forwards all paths (logs warning for unknown ext); consolidated CSS import insert logic; exported fetchTableStats.
Parser & Tests src/parser.ts, src/parser.test.ts, src/css-parser.ts	Added JSX-scoped tracking and adjusted component heuristic to require JSX or hooks; updated/added tests for positive/negative cases; removed analyzeDependencies: true from Lightning CSS transform.
Database & SQL layer src/db.ts, src/sqlite-db.ts	closeDb gains optional { readonly?: boolean } to skip PRAGMAs on readonly close; added Map-based prepared-statement cache for better-sqlite3 and hoisted inner.query() reuse.
Worker & Error Handling src/worker-pool.ts	Added worker.onerror handler for Bun workers to reject the pending promise and terminate the worker on crash.
Misc / Cleanup src/glob-sync.ts, fixtures and golden files (fixtures/golden/*, fixtures/minimal/*)	Removed an ESLint suppression comment; added multiple new golden fixtures and minimal fixture files to cover CSS, markers, exports, components and counts; small test/fixture updates.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant CLI as CLI (--files)
participant Indexer as Index Engine
participant Parser as Parser/CSS Parser
participant Worker as Worker Pool
participant DB as SQLite DB (better-sqlite3)
CLI->>Indexer: pass target file paths (all extensions)
Indexer->>Parser: parse files, collect symbols/components/css imports
Parser->>Worker: offload heavy parsing (Bun/Node)
Worker->>Indexer: return parse results (onerror handled -> reject/terminate)
Indexer->>DB: insert/update files, batch insert CSS imports
Indexer->>DB: use cached prepared statements via Map
DB-->>Indexer: confirm writes / updated stats
Indexer->>DB: closeDb({ readonly: true }) for read-only helpers (skip PRAGMA)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• chore: npm agent templates vs dev .agents, patch changeset #6: Edits the same templates/agents/skills/codemap/SKILL.md documentation template; likely related to schema/doc wording updates.

Poem

🐰 Hopping through commits with a curious twitch,
I cached prepared statements and fixed the glitch.
Components now need JSX to make the leap,
Bun workers wake up — no more silent sleep.
SHA-256 hums as old hashes take flight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: fixing three HIGH-severity bugs, implementing performance improvements, updating documentation, and performing cleanup work.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/audit-findings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

.agents/skills/codemap/SKILL.md (1)

66-85: ⚠️ Potential issue | 🟡 Minor

The skill schema still advertises a symbols.kind value the index never writes.

While updating this schema table, symbols.kind still includes variable, but src/parser.ts emits function, const, class, interface, type, and enum, and docs/architecture.md was already corrected. Please update this copy and templates/agents/skills/codemap/SKILL.md together so agent-authored SQL does not target a stale enum.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.agents/skills/codemap/SKILL.md around lines 66 - 85, The schema docs still
list a stale `symbols.kind` value `variable`; update the documentation to match
the actual enum emitted by the indexer/parser by removing `variable` and
ensuring the allowed kinds are exactly `function`, `const`, `class`,
`interface`, `type`, and `enum`; edit .agents/skills/codemap/SKILL.md (and the
template at templates/agents/skills/codemap/SKILL.md) to remove `variable` from
the table and align wording with src/parser.ts and docs/architecture.md so
generated SQL and agent prompts reference the correct enum values.

docs/architecture.md (1)

437-459: ⚠️ Potential issue | 🟡 Minor

Update the SQLite performance paragraph to reflect statement caching on Node.

Line 439 states "on Node, better-sqlite3 re-prepares on each call via the wrapper's run() method"—but src/sqlite-db.ts now implements stmtCache and cachedPrepare() to reuse prepared statements for both run() and query() on Node. Update this paragraph to clarify that the wrapper caches statements on Node, not re-prepares them.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/architecture.md` around lines 437 - 459, The doc incorrectly states that
on Node `better-sqlite3` re-prepares statements on each call; update the
"bun:sqlite API" paragraph to reflect that `src/sqlite-db.ts` now caches
prepared statements for Node using `stmtCache` and `cachedPrepare()`, so both
`run()` and `query()` reuse prepared statements instead of re-preparing them.
Edit the sentence referencing Node to mention `stmtCache`/`cachedPrepare()` and
that the wrapper reuses cached statements for `run()` and
`.query().all()`/`.get()` to match the current implementation.

src/application/run-index.ts (1)

42-44: ⚠️ Potential issue | 🟡 Minor

Update RunIndexOptions.files docstring to match current behavior.

Line 43 says non-indexable extensions are filtered out, but mode === "files" now forwards all provided paths. This comment is now inaccurate.

Suggested doc fix

   /**
    * Paths relative to the project root; used only when `mode === "files"`.
-   * Non-indexable extensions are filtered out.
+   * Paths are passed through as-is; non-standard extensions are indexed as text.
    */
   files?: string[];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/application/run-index.ts` around lines 42 - 44, Update the
RunIndexOptions.files docstring to reflect current behavior: note that when mode
=== "files" the array of paths is forwarded verbatim (no automatic filtering of
non-indexable extensions) and that callers are responsible for providing only
desired paths or performing any extension filtering themselves; reference
RunIndexOptions.files and the mode === "files" behavior in the text so it is
clear to future readers.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/application/index-engine.ts`:
- Around line 129-134: The incremental filter currently only keeps files whose
extension is in LANG_MAP or already indexed (indexedPaths from
getAllFileHashes), which skips newly added files with custom extensions; update
the predicate used for allChanged (which filters [...diffFiles, ...statusFiles])
so it also accepts files that match configured custom extensions or otherwise
should be treated as source (e.g., a helper function isCustomExtension(file)
that checks the project's custom extensions list or workspace file existence),
i.e., change the condition from "ext in LANG_MAP || indexedPaths.has(f)" to "ext
in LANG_MAP || indexedPaths.has(f) || isCustomExtension(f)" (or equivalent
logic) so new custom-extension files are included for indexing.

In `@src/db.ts`:
- Around line 20-25: The PRAGMA calls in closeDb can throw and prevent
db.close() from running; wrap the PRAGMA block (the db.run("PRAGMA
analysis_limit = 400") and db.run("PRAGMA optimize") calls) in a try and move
db.close() into a finally so the CodemapDatabase handle is always closed even on
error; preserve the opts?.readonly short-circuit (skip PRAGMAs when readonly)
and make sure any error from the PRAGMA block is not silently swallowed (rethrow
after closing or let it bubble).

---

Outside diff comments:
In @.agents/skills/codemap/SKILL.md:
- Around line 66-85: The schema docs still list a stale `symbols.kind` value
`variable`; update the documentation to match the actual enum emitted by the
indexer/parser by removing `variable` and ensuring the allowed kinds are exactly
`function`, `const`, `class`, `interface`, `type`, and `enum`; edit
.agents/skills/codemap/SKILL.md (and the template at
templates/agents/skills/codemap/SKILL.md) to remove `variable` from the table
and align wording with src/parser.ts and docs/architecture.md so generated SQL
and agent prompts reference the correct enum values.

In `@docs/architecture.md`:
- Around line 437-459: The doc incorrectly states that on Node `better-sqlite3`
re-prepares statements on each call; update the "bun:sqlite API" paragraph to
reflect that `src/sqlite-db.ts` now caches prepared statements for Node using
`stmtCache` and `cachedPrepare()`, so both `run()` and `query()` reuse prepared
statements instead of re-preparing them. Edit the sentence referencing Node to
mention `stmtCache`/`cachedPrepare()` and that the wrapper reuses cached
statements for `run()` and `.query().all()`/`.get()` to match the current
implementation.

In `@src/application/run-index.ts`:
- Around line 42-44: Update the RunIndexOptions.files docstring to reflect
current behavior: note that when mode === "files" the array of paths is
forwarded verbatim (no automatic filtering of non-indexable extensions) and that
callers are responsible for providing only desired paths or performing any
extension filtering themselves; reference RunIndexOptions.files and the mode ===
"files" behavior in the text so it is clear to future readers.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 687b739c-2875-41f0-96dd-9029df3ee454

📥 Commits

Reviewing files that changed from the base of the PR and between 24ab43f and c5aa7b9.

📒 Files selected for processing (14)

• .agents/skills/codemap/SKILL.md
• .changeset/audit-findings.md
• docs/architecture.md
• src/application/index-engine.ts
• src/application/run-index.ts
• src/cli/cmd-index.ts
• src/css-parser.ts
• src/db.ts
• src/glob-sync.ts
• src/parser.test.ts
• src/parser.ts
• src/sqlite-db.ts
• src/worker-pool.ts
• templates/agents/skills/codemap/SKILL.md

💤 Files with no reviewable changes (2)

• src/glob-sync.ts
• src/css-parser.ts

[coderabbitai]

🧹 Nitpick comments (2)

fixtures/minimal/src/styles/button.module.css (1)

8-8: Use kebab-case for keyframe name to satisfy lint rule.

Line 8 uses fadeIn, but keyframes-name-pattern expects kebab-case. Rename to fade-in (and update any fixture expectations referencing this name).

Proposed change

-@keyframes fadeIn {
+@keyframes fade-in {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@fixtures/minimal/src/styles/button.module.css` at line 8, The keyframe name
"fadeIn" in button.module.css violates the keyframes-name-pattern lint rule;
rename the keyframe to kebab-case "fade-in" in the `@keyframes` declaration and
update all references to that animation name (e.g., within button.module.css and
any fixture expectation files that assert on "fadeIn") so they use "fade-in"
instead to keep lint and tests consistent.

src/application/run-index.ts (1)

42-44: Don’t hard-code the text-fallback contract in this layer.

Line 43 now promises that non-standard extensions are indexed as text, but Lines 82-95 only forward raw paths into targetedReindex(). The provided src/application/index-engine.ts:252-320 and src/application/index-engine.ts:427-446 snippets show no extension handling downstream either, so this shared CLI and Codemap#index entry point is documenting behavior it doesn’t enforce. Consider moving the warning/fallback into runCodemapIndex()/targetedReindex() or softening the docstring so both entry points keep the same contract.

Also applies to: 82-95

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/application/run-index.ts` around lines 42 - 44, The docstring in
run-index.ts incorrectly promises a text-fallback for non-standard extensions
even though neither runCodemapIndex nor targetedReindex (and the Codemap#index
entry path) enforce that behavior; either remove/soften that assertion in the
comment OR implement the fallback in the runtime: add extension-to-text handling
inside runCodemapIndex() or targetedReindex() so paths with non-standard
extensions are normalized to the text indexing branch before calling
Codemap#index; reference runCodemapIndex, targetedReindex, and Codemap#index
when making the change so both the CLI surface and the shared index entrypoint
have a single, consistent contract.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@fixtures/minimal/src/styles/button.module.css`:
- Line 8: The keyframe name "fadeIn" in button.module.css violates the
keyframes-name-pattern lint rule; rename the keyframe to kebab-case "fade-in" in
the `@keyframes` declaration and update all references to that animation name
(e.g., within button.module.css and any fixture expectation files that assert on
"fadeIn") so they use "fade-in" instead to keep lint and tests consistent.

In `@src/application/run-index.ts`:
- Around line 42-44: The docstring in run-index.ts incorrectly promises a
text-fallback for non-standard extensions even though neither runCodemapIndex
nor targetedReindex (and the Codemap#index entry path) enforce that behavior;
either remove/soften that assertion in the comment OR implement the fallback in
the runtime: add extension-to-text handling inside runCodemapIndex() or
targetedReindex() so paths with non-standard extensions are normalized to the
text indexing branch before calling Codemap#index; reference runCodemapIndex,
targetedReindex, and Codemap#index when making the change so both the CLI
surface and the shared index entrypoint have a single, consistent contract.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9a88a4c9-73bd-4bfe-afba-3b6a505b02f4

📥 Commits

Reviewing files that changed from the base of the PR and between c5aa7b9 and 9b323fd.

📒 Files selected for processing (22)

• .agents/skills/codemap/SKILL.md
• .changeset/audit-findings.md
• docs/architecture.md
• docs/benchmark.md
• docs/golden-queries.md
• fixtures/golden/minimal/components-no-false-positives.json
• fixtures/golden/minimal/css-classes-module.json
• fixtures/golden/minimal/css-imports.json
• fixtures/golden/minimal/css-keyframes.json
• fixtures/golden/minimal/css-variables.json
• fixtures/golden/minimal/exports-client.json
• fixtures/golden/minimal/files-count.json
• fixtures/golden/minimal/index-summary.json
• fixtures/golden/minimal/markers-all-kinds.json
• fixtures/golden/scenarios.json
• fixtures/minimal/src/components/shop/ShopButton.tsx
• fixtures/minimal/src/consumer.ts
• fixtures/minimal/src/styles/button.module.css
• fixtures/minimal/src/theme.css
• src/application/run-index.ts
• src/db.ts
• templates/agents/skills/codemap/SKILL.md

✅ Files skipped from review due to trivial changes (15)

• fixtures/golden/minimal/components-no-false-positives.json
• fixtures/minimal/src/consumer.ts
• fixtures/golden/minimal/files-count.json
• fixtures/golden/minimal/css-classes-module.json
• fixtures/golden/minimal/markers-all-kinds.json
• fixtures/minimal/src/theme.css
• fixtures/golden/minimal/index-summary.json
• fixtures/golden/minimal/css-imports.json
• docs/golden-queries.md
• fixtures/golden/minimal/exports-client.json
• fixtures/golden/minimal/css-variables.json
• fixtures/golden/scenarios.json
• fixtures/golden/minimal/css-keyframes.json
• docs/benchmark.md
• .changeset/audit-findings.md

🚧 Files skipped from review as they are similar to previous changes (3)

• templates/agents/skills/codemap/SKILL.md
• src/db.ts
• docs/architecture.md