Skip to content

docs: refresh CONTRIBUTING.md for TypeScript architecture and current tooling#1977

Open
carlos-alm wants to merge 1 commit into
fix/issue-1830-follow-up-document-or-discriminate-file-levelfrom
fix/issue-1831-contributing-md-is-stale-references-removed
Open

docs: refresh CONTRIBUTING.md for TypeScript architecture and current tooling#1977
carlos-alm wants to merge 1 commit into
fix/issue-1830-follow-up-document-or-discriminate-file-levelfrom
fix/issue-1831-contributing-md-is-stale-references-removed

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

CONTRIBUTING.md described the pre-TypeScript codebase and was stale on every point raised in #1831:

  • Project Structure listed src/cli.js, src/parser.js, src/builder.js, src/queries.js, src/embedder.js, src/db.js, src/mcp.js, src/cycles.js, src/export.js, src/watcher.js, src/config.js, src/constants.js — none of these exist anymore. Rewritten to reflect the current TypeScript layer-based tree (shared/, infrastructure/, db/, domain/, extractors/, features/, presentation/, graph/, mcp/, ast-analysis/).
  • Code Style said "All source is plain JavaScript (ES modules) — no transpilation" — fixed to describe TypeScript + tsc/tsup.
  • Requirements: Node.js >= 20 — fixed to >= 22.12.0, matching package.json's engines.node (the actual enforced minimum).
  • Architecture Notes said "The feat/rust-core branch introduces an optional native Rust engine" — that branch merged long ago; rewritten to describe the current default dual-engine architecture (--engine native|wasm|auto).
  • Test file examples used .test.js — fixed to .test.ts throughout (Commands, Parser Improvements, resolution benchmark sections).
  • Database section listed only nodes, edges, metadata, embeddings — now defers to CLAUDE.md's Database entry instead of duplicating a table list that will go stale again.
  • WASM Grammars section claimed .wasm files are "pre-built and committed" — corrected: they're built from devDependencies via the prepare npm script on every npm install, not committed (except one pinned exception already noted elsewhere in the doc's worktree guidance).
  • Regression Benchmarks table and script paths (scripts/benchmark.js, scripts/embedding-benchmark.js, etc.) updated to their current .ts locations and invocation commands.

Architectural choice

Rather than re-duplicating the full module-by-module architecture table in CONTRIBUTING.md (which is exactly how it went stale the first time), the Project Structure and Architecture Notes sections now give a short orientation and explicitly point to CLAUDE.md's Architecture section as the actively-maintained source of truth. CLAUDE.md is kept in sync with the codebase as a matter of course; a second full copy in CONTRIBUTING.md would just create a second thing to keep in sync.

Out-of-scope findings filed separately

While researching this fix I found two more pre-existing staleness issues, filed separately per repo convention rather than expanding this PR's scope:

Test plan

  • npm run lint — clean (846 files, no issues; CONTRIBUTING.md is outside Biome's src//tests/ scope)
  • npm run typecheck — clean
  • codegraph diff-impact --staged -T — no function-level changes (docs-only, as expected)
  • npm run doctor — environment healthy after fresh worktree npm install
  • Manually cross-checked every remaining .js/stale-path/stale-version reference against the current source tree, package.json, and CLAUDE.md

Fixes #1831

… tooling

CONTRIBUTING.md described the pre-TypeScript codebase: a plain-JS src/ tree
with files that no longer exist (cli.js, builder.js, parser.js, queries.js,
embedder.js, db.js, mcp.js, cycles.js, export.js, watcher.js, config.js,
constants.js), Node >= 20 (actual requirement is >= 22.12.0 per package.json),
an experimental "feat/rust-core" branch for the native engine (long since
merged and now the default), .test.js examples (suite is .test.ts), and a
stale WASM-grammars-are-committed claim (they're built from devDependencies
via the prepare script on every npm install, not committed).

Rewrite Project Structure, Architecture Notes, Code Style, Commands, Testing,
Regression Benchmarks, Parser Improvements, and WASM Grammars sections to
match the current TypeScript layer-based src/ tree, dual-engine architecture,
and tooling. Project Structure and Architecture Notes now point to CLAUDE.md
for the module-by-module breakdown instead of duplicating it, so this doc
can't drift out of sync with the codebase the same way again.
@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refreshes the contributor guide for the current TypeScript and native-engine setup. The main changes are:

  • Updated Node.js, project structure, and code style guidance.
  • Replaced stale JavaScript test and benchmark examples with TypeScript paths.
  • Pointed architecture and database details at CLAUDE.md.
  • Updated WASM grammar setup and rebuild instructions.

Confidence Score: 5/5

This looks safe to merge after two small documentation fixes.

  • The PR only changes contributor documentation.
  • Most updated commands and paths match the current repository.
  • Two changed statements can mislead contributors about the build tool and the tracked WASM grammar exception.

CONTRIBUTING.md

Important Files Changed

Filename Overview
CONTRIBUTING.md Updates contributor documentation for the current TypeScript architecture, tooling, benchmark commands, dual-engine behavior, and WASM grammar workflow.

Fix All in Claude Code

Reviews (1): Last reviewed commit: "docs: refresh CONTRIBUTING.md for TypeSc..." | Re-trigger Greptile

Comment thread CONTRIBUTING.md
Comment on lines +61 to +62
Source is TypeScript under `src/`, compiled via `tsup`/`tsc`; the native engine
lives in `crates/codegraph-core/` (Rust, via napi-rs) and mirrors the `src/`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Build Tool Name Is Wrong

This says the TypeScript source is compiled with tsup, but the documented build path uses tsc and tsup is not part of the repository scripts. A contributor following this can look for or invoke the wrong build tool instead of the actual npm run build path.

Suggested change
Source is TypeScript under `src/`, compiled via `tsup`/`tsc`; the native engine
lives in `crates/codegraph-core/` (Rust, via napi-rs) and mirrors the `src/`
Source is TypeScript under `src/`, compiled via `tsc`; the native engine
lives in `crates/codegraph-core/` (Rust, via napi-rs) and mirrors the `src/`

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

Comment thread CONTRIBUTING.md
Comment on lines +330 to +333
`.wasm` grammar files are **not** committed to git — they're built from
`devDependencies` into `grammars/` automatically via the `prepare` npm script
(`npm run build:wasm`), which runs on every `npm install`. Each git worktree
therefore needs its own `npm install` before its `grammars/` directory is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Tracked Grammar Exception Hidden

This says .wasm grammar files are not committed, but the repo still tracks the pinned grammars/tree-sitter-erlang.wasm exception. A contributor can treat that file as generated-only and remove or ignore a required tracked artifact.

Suggested change
`.wasm` grammar files are **not** committed to git — they're built from
`devDependencies` into `grammars/` automatically via the `prepare` npm script
(`npm run build:wasm`), which runs on every `npm install`. Each git worktree
therefore needs its own `npm install` before its `grammars/` directory is
Most `.wasm` grammar files are **not** committed to git — they're built from
`devDependencies` into `grammars/` automatically via the `prepare` npm script
(`npm run build:wasm`), which runs on every `npm install`; pinned exceptions
such as `grammars/tree-sitter-erlang.wasm` remain tracked. Each git worktree
therefore needs its own `npm install` before its `grammars/` directory is

Fix in Claude Code

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.

1 participant