Skip to content

chore: bump tree-sitter runtime to 0.25 (python/go grammars to match) - #40

Open
qoole wants to merge 1 commit into
trailhq:mainfrom
qoole:feat/tree-sitter-0.25
Open

chore: bump tree-sitter runtime to 0.25 (python/go grammars to match)#40
qoole wants to merge 1 commit into
trailhq:mainfrom
qoole:feat/tree-sitter-0.25

Conversation

@qoole

@qoole qoole commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Why

Newer tree-sitter grammars are published against the 0.25 node runtime — every published PowerShell grammar (which we'd like to propose as a new language in a follow-up PR) and the current python/go grammar lines peer-depend on tree-sitter@^0.25. The bump also picks up two years of runtime fixes; among them, the string-parse() size limit that extract.ts's chunked callback works around no longer exists in 0.25 (comment updated; behavior kept).

What

  • tree-sitter ^0.21.1 → ^0.25.0; tree-sitter-python, tree-sitter-go → ^0.25.0. tree-sitter-typescript stays at ^0.23.2 (its latest release).
  • overrides entry for tree-sitter-typescript: its latest release still declares peerOptional tree-sitter@^0.21, which hard-fails a root-project npm install under npm's peer resolution. The override resolves the repo's own install (and npm ci in CI) cleanly. Empirically verified: the 0.23.2 grammar binding loads and parses correctly under runtime 0.25.1.
  • dependabot: ignore tree-sitter-typescript (bumping it independently re-breaks peer resolution), with an explanatory comment.

Evidence

  • Full suite green with zero code changes.
  • Extraction output is byte-identical before/after on two real-world repos (psf/requests: 844 nodes / 1790 edges; spf13/afero: 919 / 2263) — every card, wiring.json, and cache payload; only the extractor fingerprint id changes (intended — it keys cache invalidation).
  • Consumer installs verified from a packed tarball with no flags: local dependency install and global npm i -g both resolve. npm nests a vestigial tree-sitter@0.21 under tree-sitter-typescript (overrides don't propagate to consumers); the CLI was verified to build and query TS/Python/Go correctly in exactly that layout. The nesting disappears once tree-sitter-typescript ships a 0.25-peer release.

@anirudhkumar-nanonets

Copy link
Copy Markdown
Collaborator

Thanks, this is valuable and we’d like to move it forward. Recent changes to main introduced conflicts. Could you rebase onto the latest main, resolve them, and rerun the test suite? We’ll review it promptly afterward.

@qoole
qoole force-pushed the feat/tree-sitter-0.25 branch from fa7f604 to 20e9357 Compare August 5, 2026 16:50
@qoole

qoole commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (c2cc532) and re-ran the suite — 538/538 green. The only conflict was CHANGELOG placement; the bump itself applied cleanly over the new walker and posix-path work.

@anirudhkumar-nanonets anirudhkumar-nanonets left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed after the rebase and follow-up compatibility fix. Ubuntu, Windows, and CodeQL checks are green.

@qoole

qoole commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Merged main (0.9.0) into the branch — the only conflict was CHANGELOG placement, and the bump entry (including the stable-node-ID and Node 22.12 notes from 8899d26) now sits under a fresh Unreleased heading above the 0.9.0 section. Suite is 583/583 green on the merged tree, and the lockfile version header is synced. Should be conflict-free to merge — thanks for the compatibility fix.

Assaf750 added a commit to Assaf750/Graft that referenced this pull request Aug 12, 2026
…nd Rust behaviour

Records the verified starting point before any Rust work: PR trailhq#40/trailhq#58/trailhq#59 heads
double-checked via the GitHub API and git ls-remote, strict-containment proof of
the stack, and the finding that the stack carries no upstream CI signal at all.

Suite on the pinned head is 638/645 with 3 failures. All three are an upstream
locale defect (bare toLocaleString() resolving to ar-SA digits), reproduced
verbatim on main and untouched by any Rust commit, so they are attributed
upstream rather than counted as a Rust regression. Fix is deliberately deferred
to its own commit.

Also records measured behaviour on a Cargo workspace fixture: what already
resolves, and the six gaps that Batches B-J exist to close — chiefly that
cross-crate symbol imports never resolve because module-path resolution maps
paths to files only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@qoole

qoole commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Status note — this has sat since 6 August and that's on me. Short version: one piece of it is now redundant, the rest still isn't covered anywhere, and the overrides block needs widening before it installs on today's main.

Redundant. The only behavioural source change here — comparing SyntaxNode ids instead of object identity — landed via #120, with a third call site (withoutShadowedImports), a determinism test, and a sameSyntaxNode helper that compares a.tree too, which mine didn't. That hunk gets dropped on rebase, not carried. What's left is dependency and config: the runtime plus the matching python/go grammar bumps, the overrides block, the dependabot ignore for tree-sitter-typescript, engines.node, CI node 22, a CHANGELOG entry, and a docblock correction in extract.ts noting 0.25 lifted the 32 KB string-parse() limit the chunked callback works around.

Still needed. Main is on tree-sitter@^0.21.1 and the depth tier is entirely native, so the pin is still the binding constraint — tree-sitter-python@0.25.0 and tree-sitter-go@0.25.0 both peer on ^0.25.0. Dependabot has the bumps in pieces (#28, #19, #15), but none carries the overrides block and none lands alone: #28's ubuntu job fails ERESOLVE on peerOptional tree-sitter@"^0.21.1" from tree-sitter-go@0.23.4, and #19 is conflicting. Being straight about #164, since it's the obvious thing to cite and it doesn't hold up: ^0.22.4 doesn't accept 0.25.1 either, so that one needs an overrides entry whichever runtime we land on — evidence the peer graph needs a policy, not evidence this PR fixes it. And on #119, @Frankie-Xu discounted the bump — fairly, given the state of this branch, and because Alpine/musl loses either way — and went with the WASM fallback in #214. That narrows the pin's blast radius; it doesn't retire it.

Needs widening. This branch was cut when there were four tree-sitter deps. Main now has eight native grammar packages, and after this PR's own python/go bumps, five are still on the 0.21 peer range: tree-sitter-java@0.23.5, tree-sitter-kotlin@0.3.8, tree-sitter-php, tree-sitter-r (an npm alias for @davisvaughan/tree-sitter-r, so not the obvious overrides key), and tree-sitter-javascript nested under tree-sitter-typescript, which the single existing override doesn't reach. Java, Kotlin and R are at their latest published release, so there's no bump path for those three at all. Main also gained an allowScripts map keyed by exact name@version after this branched — those keys have to be rewritten alongside the bumps or the install scripts are skipped and the native builds never happen.

I've now checked the part I was least sure of, since an overrides entry only silences npm's resolver and doesn't make a native addon ABI-compatible. On a scratch install with a single hoisted tree-sitter@0.25.1 (Linux x64, Node 26), all seven grammars load and parse clean: python 0.25.0, go 0.25.0, java 0.23.5, kotlin 0.3.8, php 0.23.12, r 1.3.0 and typescript 0.23.2. So the five that stay on the 0.21 peer range work under the new runtime — the peer metadata is stale, not the binaries.

Two practical notes from that run. The overrides key for R has to be tree-sitter-r, the alias, not @davisvaughan/tree-sitter-r — keyed on the real package name npm ignores it and still fails ERESOLVE. And tree-sitter-kotlin ships no prebuilds for any platform, so it compiles from source everywhere; it built fine here, but it's the one to watch on the Windows leg, and I'd want CI green on all three OSes before this is worth your time. I haven't re-run the parse-drift comparison for the two grammars whose versions actually change (python, go) against current main, so that's still outstanding.

All of which makes the result materially different from what was approved on 6 August, so please treat that approval as lapsed.

On the Node floor, I'll take it out of this PR — it turns out not to belong here. tree-sitter@0.25.1 declares no engines at all and its prebuilds are untagged N-API, so the runtime imposes no floor; the 22.12 is Commander's, as the CHANGELOG entry on the branch already says. Which surfaces something independent of this PR: main's lockfile pins commander@15.0.0, whose own engines is >=22.12.0, while main's package.json declares >=20 and CI tests on Node 20. That's a live mismatch today, with or without the runtime bump, so I'll send engines.node and the CI bump as their own small PR on that reasoning and leave this one purely about the peer graph.

One other thing to decide: if a fresh cut against current main reviews better than rebasing this branch, say so and I'll do that instead. Either way I'll post the SHA and the suite count once it's pushed.

@qoole
qoole force-pushed the feat/tree-sitter-0.25 branch from b25adaa to 1a9f5d0 Compare August 24, 2026 19:55
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
@qoole

qoole commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Pushed as 1a9f5d0, cut fresh against ee1ef03 rather than replaying the old branch — it carried a merge commit and a lot of drift, and the residual is small enough that a clean cut reviews better. Now MERGEABLE. Five files: the three dependency bumps, the overrides block, the allowScripts keys, the dependabot ignore, the PARSE_CHUNK docblock, and a CHANGELOG entry. The node-id hunk is gone, as #120 covers it.

Suite is 914 passing, 1 failing on Linux. The failure is viz --tabs: a bad tab name fails loudly…, and it fails identically on unmodified ee1ef03 — the graph-missing check runs before the tab-name validation, so the assertion gets no context graph … run graft build --deep first. Not something this PR touches.

Since the checks here are red, worth saying plainly: both legs fail the same way on main itself, so this PR adds nothing new.

On the Windows one: it's independent of the runtime version — 0.21.1 and 0.25.1 ship the same prebuild layout (prebuilds/win32-x64/tree-sitter.node), and the failure is toolchain discovery, not the addon. Which makes the CI Node bump in 8899d26 look worth keeping for its own reasons: a newer Node brings a newer bundled node-gyp, and that's the piece that would have to recognise VS 18. I've kept engines.node and the CI bump out of this PR and will send them separately, since the floor itself is Commander's — commander@15 is already a dependency and declares >=22.12.0 while package.json says >=20. Happy to fold them back in here instead if you'd rather they land together, given the Windows leg can't go green without something in that direction.

Verification on the bump itself, all cold-cache:

  • All seven native grammars load and parse under tree-sitter@0.25.1 — python 0.25.0, go 0.25.0, java 0.23.5, kotlin 0.3.8, php 0.23.12, r 1.3.0, typescript 0.23.2. The stale ^0.21 peer metadata on five of them doesn't reflect an ABI break.
  • npm ls tree-sitter --all resolves every consumer to a single tree-sitter@0.25.1, tree-sitter-javascript included — the nested override on tree-sitter-typescript reaches it, so it needs no entry of its own.
  • Graph output is byte-identical to 0.21.1 on psf/requests (37 python files) and spf13/afero (64 go files) — same nodes, same edges, same bytes. Only the extract cache key changes, which is correct: the extractor version is part of it, so an upgrade invalidates stale entries rather than replaying them. Worth flagging that a warm cache hides this entirely; the first comparison I ran replayed all 37 files and showed no difference for the wrong reason.

One wrinkle for whoever touches this next: the overrides key for R has to be tree-sitter-r, the alias. Keyed on @davisvaughan/tree-sitter-r npm ignores it and ERESOLVE still fires.

qoole added a commit to qoole/Graft that referenced this pull request Aug 24, 2026
commander@15 is already a dependency and declares engines.node >=22.12.0,
while this package declared >=20 and CI tested on Node 20. A Node 20 install
therefore satisfied the stated range and then ran an unsupported Commander.

Raise the floor to >=22.12.0 and move the CI matrix to Node 22 to match.

Independent of the tree-sitter runtime work in trailhq#40 — tree-sitter@0.25.1
declares no engines of its own and ships untagged N-API prebuilds, so it
imposes no Node floor.
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
@qoole

qoole commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Folded the engines.node / CI-Node change in here as 342025e rather than sending it separately, on the reasoning in my last comment. That reasoning was wrong, so recording the result: moving CI to Node 22 does not fix the Windows leg. Node 22.23.2 ships node-gyp 11.5.0, and 11.5.0 fails to detect VS 18 exactly as 10.1.0 did — unknown version "undefined" found at "C:\Program Files\Microsoft Visual Studio\18\Enterprise". The commit is still right on its own merits (commander@15 really does require >=22.12.0), just not for the reason I gave. Happy to pull it back out if you'd rather it travelled alone.

The actual root cause, which I should have found first: tree-sitter-kotlin ships no prebuilds for any platform. Every other native grammar here has them, so npm ci resolves those from prebuilds/ without a toolchain; kotlin has to compile, and compiling on the current windows-latest image is impossible for any Node version, because no shipped node-gyp recognises VS 18. gyp ERR! cwd D:\a\Graft\Graft\node_modules\tree-sitter-kotlin. Nothing to do with the runtime version — tree-sitter-kotlin@0.3.8 is on main today and main's own Windows leg fails identically.

So this PR can't show green while main isn't, and both red checks are main's:

  • windows-latest — pinning the matrix to windows-2022 (VS 2022, which node-gyp handles) would unbreak it until either kotlin ships prebuilds or node-gyp learns VS 18.
  • ubuntu-latest — the viz --tabs ordering bug, 914/1 on main and here alike. The graph-existence check runs before the argument validation, so the test's expected error is unreachable.

Both are one-liners and neither belongs in a dependency PR. Happy to send either or both as their own PR if useful — just say which. Otherwise I'll leave them alone, and this stays as it is: MERGEABLE, no new failures relative to main.

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

🌱 graft blast radius

1 area changed → 5 areas can be affected. 18 dependent symbols, depth 2.

flowchart TB
  A0(("Dependency Graph<br/>13 symbols"))
  A1(("CLI Engine<br/>2 symbols"))
  A2(("LSP Enrichment<br/>1 symbol"))
  A3(("Code Review<br/>1 symbol"))
  A4(("MCP Tools<br/>1 symbol"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0,A1,A2,A3,A4 reached;
Loading
Can be affected Symbols Nearest hop Reached from
Dependency Graph 13 src/graph/bindings.ts:L1-L615 bindings.ts — imports, depth 1 Dependency Extraction
CLI Engine 2 src/cli.ts:L1-L1033 cli.ts — imports, depth 2 Dependency Extraction
LSP Enrichment 1 src/graph/lsp/enrich.ts:L1-L141 enrich.ts — imports, depth 1 Dependency Extraction
Code Review 1 src/app/review.ts:L1-L159 review.ts — imports, depth 2 Dependency Extraction
MCP Tools 1 src/mcp/tools.ts:L1-L339 tools.ts — imports, depth 2 Dependency Extraction
All 18 dependent symbols, grouped by area

Dependency Graph — 13 symbols in 13 files

  • src/graph/bindings.ts:L1-L615 — bindings.ts (imports, depth 1)
    12: import type { Language, WalkCtx } from "./extract.js";
  • src/graph/build.ts:L1-L404 — build.ts (imports, depth 1)
    20: import { extractFile, languageLabelOf, languageOf, type RawEdge } from "./extract.js";
  • src/graph/check.ts:L1-L190 — check.ts (imports, depth 1)
    23: import { extractFile, languageOf } from "./extract.js";
  • src/graph/container.ts:L1-L209 — container.ts (imports, depth 1)
    24: import { extractFile, mintId, type ExtractResult, type Language, type RawEdge } from "./extract.js";
  • src/graph/extract-cache.ts:L1-L224 — extract-cache.ts (imports, depth 1)
    2: * Per-file memo for Tier-1 extraction — `<outDir>/.cache/extract.json`.
  • src/graph/generic.ts:L1-L527 — generic.ts (imports, depth 1)
    26: import type { ExtractResult, RawEdge } from "./extract.js";
  • src/graph/map.ts:L1-L351 — map.ts (imports, depth 1)
    22: import { languageLabelOf } from "./extract.js";
  • src/graph/resolve.ts:L1-L614 — resolve.ts (imports, depth 1)
    18: import { languageOf, type RawEdge } from "./extract.js";
  • src/graph/source-files.ts:L1-L94 — source-files.ts (imports, depth 1)
    14: import { languageOf, depthExtensions } from "./extract.js";
  • src/graph/fingerprint.ts:L1-L185 — fingerprint.ts (imports, depth 2)
    46: * `extract.json` carries. Without it the two sidecars can disagree about whether
  • src/graph/refresh.ts:L1-L270 — refresh.ts (imports, depth 2)
  • src/graph/seed.ts:L1-L229 — seed.ts (imports, depth 2)
  • src/graph/workspace.ts:L1-L509 — workspace.ts (imports, depth 2)

CLI Engine — 2 symbols in 2 files

  • src/cli.ts:L1-L1033 — cli.ts (imports, depth 2)
  • src/engine.ts:L1-L155 — engine.ts (imports, depth 2)

LSP Enrichment — 1 symbol in 1 file

  • src/graph/lsp/enrich.ts:L1-L141 — enrich.ts (imports, depth 1)
    16: import { languageLabelOf } from "../extract.js";

Code Review — 1 symbol in 1 file

  • src/app/review.ts:L1-L159 — review.ts (imports, depth 2)

MCP Tools — 1 symbol in 1 file

  • src/mcp/tools.ts:L1-L339 — tools.ts (imports, depth 2)
Test signal per changed area — 1 –

Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.

  • Dependency Extraction — 8 test files reach it, none changed here
37 test suites also reference this code

37 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.

  • test/ask-index.test.ts
  • test/ask.test.ts
  • test/container-extract.test.ts
  • test/covers.test.ts
  • test/generic-extract.test.ts
  • test/graph-bindings.test.ts
  • test/graph-cross-language.test.ts
  • test/graph-enrich-pending.test.ts
  • test/graph-extract-dedup.test.ts
  • test/graph-go.test.ts
  • test/graph-incremental.test.ts
  • test/graph-invariants.test.ts
  • test/graph-java.test.ts
  • test/graph-languages.test.ts
  • test/graph-map.test.ts
  • test/graph-php.test.ts
  • test/graph-posix-paths.test.ts
  • test/graph-python.test.ts
  • test/graph-r-classes.test.ts
  • test/graph-r-phase3.test.ts
  • …17 more

⚠️ 4 changed files not in the graph (.github/dependabot.yml, CHANGELOG.md, package-lock.json, package.json) — no parser claims the extension, or the index predates the file.

graft blast · origin/main...HEAD · depth 2 · 5 changed files

Open the interactive graph → — click an area to see its dependent symbols at file:line.

github-actions Bot added a commit that referenced this pull request Aug 25, 2026
github-actions Bot added a commit that referenced this pull request Aug 25, 2026
Pin the five grammars that still declare the ^0.21 peer range — java, kotlin,
php, r and typescript — onto the single hoisted runtime with an overrides
block, so the tree resolves to exactly one copy of tree-sitter. All seven
grammars were verified to load and parse under 0.25.1; the stale peer metadata
does not reflect an ABI break.

The overrides key for R must be the alias tree-sitter-r, not
@davisvaughan/tree-sitter-r, or npm ignores it and ERESOLVE still fires.

allowScripts is keyed by exact name@version, so the three moved versions are
rewritten alongside the bumps to keep the list in sync.

Dependabot is told not to bump tree-sitter-typescript on its own.

Also corrects two docblocks that named the wrong runtime: the string parse()
size limit was lifted in 0.22.0 (node-tree-sitter 9ea6558), not 0.25; and the
PHP collapsed-enum workaround no longer cites an ABI ceiling this bump removes.

Leaves the node-gyp 12 pin and the Node 20 matrix from trailhq#228 untouched.
@qoole
qoole force-pushed the feat/tree-sitter-0.25 branch from 342025e to 873842a Compare August 25, 2026 19:19
@qoole

qoole commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto d92f856 as 873842a, and dropped the engines.node / CI-Node commit — #228's comment makes the Node 20 pin a deliberate, documented choice, and a dependency bump is the wrong PR to quietly move it in. The commander@15 mismatch is still there (>=22.12.0 required, >=20 declared, matrix on 20) but it's your call, not a rider on this. Happy to open it as its own thing if you want it.

Thanks for #228 — that diagnosis is better than mine. I'd got as far as VS 18 being invisible to node-gyp, and wrongly assumed a newer Node would fix it via its bundled copy; Node 22 ships node-gyp 11.5.0 and fails identically. The node-gyp-build detail — plain require, so the local devDependency wins over npm's — is the part I missed, and it's the part that actually works. The pin and the Node 20 matrix are untouched here; .github/dependabot.yml is the only workflow-adjacent file in the diff.

Now green locally: 938 passing, 0 failing (#223 fixed the viz --tabs ordering, which was the one failure I was reporting as pre-existing).

Verification on this cut:

  • npm ci clean under both npm 10 (what the matrix uses) and npm 12.
  • npm ls tree-sitter --all → one hoisted tree-sitter@0.25.1, every grammar deduped, tree-sitter-javascript included.
  • Deleting the overrides block reproduces ERESOLVE on peerOptional tree-sitter@"^0.21.0" from tree-sitter-kotlin@0.3.8, so the block is load-bearing rather than decorative.
  • The Windows compile surface is unchanged by this PR: exactly one package builds from source before and after — tree-sitter-kotlin@0.3.8, which ships no prebuilds for any platform and which this PR doesn't touch. Everything else resolves from prebuilds/.
  • Graph output byte-identical to 0.21.1 on psf/requests and spf13/afero with a cold extract cache.

Two corrections folded into this push, both mine:

  • The PARSE_CHUNK docblock I rewrote credited 0.25 with lifting the 32 KB string parse() limit. It was 0.22.0 — node-tree-sitter 9ea6558, "parsing issue when string is larger than buffer length", which is in v0.22.0 and not in v0.21.1. Now says 0.22.
  • extract.ts line ~424 justified the PHP collapsed-enum workaround with "0.24.2 … is ABI 15 and cannot load on Graft's tree-sitter 0.21.1". This PR removes that ceiling, so the comment now says the workaround stands only until tree-sitter-php is bumped separately. The workaround itself is untouched.

Two things I'd rather flag than have you find:

  • The dependabot ignore is weaker than its comment claims. overrides is keyed by package name, so a tree-sitter-typescript bump would still be pinned onto the hoisted runtime and wouldn't re-break resolution — and 0.23.2 is currently the latest published anyway. It's belt-and-braces. Say the word and I'll drop it, or widen it to the whole grammar set.
  • Pre-existing on main, unrelated to this PR: the allowScripts key tree-sitter-r@1.3.0 matches nothing, because npm derives that identity from the lockfile's resolved tarball (the scoped real name), not the alias — npm ci --strict-allow-scripts reports @davisvaughan/tree-sitter-r@1.3.0 as unreviewed against main too. Harmless today only because it ships prebuilds. The fix is @davisvaughan/tree-sitter-r@1.3.0 in allowScripts; note the overrides key must stay the alias. Left it out of here — one-liner if you want it.

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