Repository navigation
Conversation
|
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. |
fa7f604 to
20e9357
Compare
|
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
left a comment
There was a problem hiding this comment.
Reviewed after the rebase and follow-up compatibility fix. Ubuntu, Windows, and CodeQL checks are green.
|
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. |
…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>
|
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 Redundant. The only behavioural source change here — comparing Still needed. Main is on 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: I've now checked the part I was least sure of, since an Two practical notes from that run. The 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. 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. |
b25adaa to
1a9f5d0
Compare
|
Pushed as Suite is 914 passing, 1 failing on Linux. The failure is Since the checks here are red, worth saying plainly: both legs fail the same way on
On the Windows one: it's independent of the runtime version — 0.21.1 and 0.25.1 ship the same prebuild layout ( Verification on the bump itself, all cold-cache:
One wrinkle for whoever touches this next: the |
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.
|
Folded the The actual root cause, which I should have found first: So this PR can't show green while
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: |
🌱 graft blast radius1 area changed → 6 areas can be affected. 19 dependent symbols, depth 2. flowchart TB
A0(("Dependency Graph<br/>13 symbols"))
A1(("CLI Engine<br/>2 symbols"))
A2(("Build Context<br/>1 symbol"))
A3(("LSP Enrichment<br/>1 symbol"))
A4(("Code Review<br/>1 symbol"))
AX(("1 smaller area<br/>1 symbol"))
classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
class A0,A1,A2,A3,A4 reached;
classDef tail fill:#EEF2F3,stroke:#9AA4A9,stroke-width:1px,color:#3A4247;
class AX tail;
Who knows this code — 3 people across 7 areas
Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no All 19 dependent symbols, grouped by areaDependency Graph — 13 symbols in 13 files
CLI Engine — 2 symbols in 2 files
Build Context — 1 symbol in 1 file
LSP Enrichment — 1 symbol in 1 file
Code Review — 1 symbol in 1 file
MCP Tools — 1 symbol in 1 file
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.
41 test suites also reference this code41 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.
Open the interactive graph → — click an area to see its dependent symbols at file:line. |
342025e to
873842a
Compare
|
Rebased onto Thanks for #228 — that diagnosis is better than mine. I'd got as far as VS 18 being invisible to Now green locally: 938 passing, 0 failing (#223 fixed the Verification on this cut:
Two corrections folded into this push, both mine:
Two things I'd rather flag than have you find:
|
873842a to
4c1edb0
Compare
|
Rebased onto Nice to see Re-verified against the new base, since Swift is a grammar I hadn't tested before:
The two docblock corrections from the last push carry over unchanged (the 32 KB One thing worth writing down, since #233 just went in and the two look inconsistent side by side: Unrelated freebie while I was in there, if you want it as a follow-up: |
Join the grammars that declare an older peer range — java, kotlin, php, r and typescript — to the existing overrides block alongside tree-sitter-swift, so the tree resolves to exactly one copy of tree-sitter. All eight 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. Note this is the opposite of allowScripts, which keys by the resolved package (trailhq#233) — the two fields match on different identities. 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.
4c1edb0 to
8feff0a
Compare
|
Rebased onto Both red checks here are main's, not this branch's:
The ask, and I'd rather put it plainly than keep quietly force-pushing: this has been approved since 6 August, and main moves fast enough that it goes conflicting every couple of days — this is the third rebase in a week. Could you either merge it or close it? Closing is a perfectly good answer; the runtime bump matters less now that the WASM breadth tier carries the long tail, and I won't be offended. If it's blocked on something I haven't spotted, say what and I'll fix it. Otherwise I'll leave it as is rather than keep rebasing. |
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 thatextract.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-typescriptstays at ^0.23.2 (its latest release).overridesentry for tree-sitter-typescript: its latest release still declarespeerOptional tree-sitter@^0.21, which hard-fails a root-projectnpm installunder npm's peer resolution. The override resolves the repo's own install (andnpm ciin CI) cleanly. Empirically verified: the 0.23.2 grammar binding loads and parses correctly under runtime 0.25.1.Evidence
npm i -gboth resolve. npm nests a vestigialtree-sitter@0.21under 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.