Skip to content

fix: clear pre-existing lint debt and unbreak the pre-commit workflow - #6

Merged
trillium merged 4 commits into
command-visualizerfrom
parlay/cursorless-lint-debt2
Aug 5, 2026
Merged

fix: clear pre-existing lint debt and unbreak the pre-commit workflow#6
trillium merged 4 commits into
command-visualizerfrom
parlay/cursorless-lint-debt2

Conversation

@trillium

@trillium trillium commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Two CI checks fail on the command-visualizer base branch for reasons unrelated to any PR diff, and reproduce on the base branch itself. This fixes both.

1. Lint — 194 oxlint warnings + ~25 unformatted files

pnpm lint failed on the base branch: oxlint --deny-warnings reported 194 warnings across 24 files in packages/command-visualizer/src/, and lint:fmt (oxfmt) flagged ~25 files.

Every warning is fixed as a real code change, not a rule disable:

Rule Fix
unicorn/no-array-for-each forEachfor..of (10 sites)
unicorn/prefer-string-replace-all, eslint/require-unicode-regexp replaceAll, /u flag on every regex
unicorn/consistent-function-scoping hoisted pct100, toPos, pctc, snapshot to module scope
unicorn/custom-error-definition ChainContinuityError now sets .name
node/no-process-env, unicorn/import-style import { env } from "node:process", namespace path import
import/no-duplicates merged split value/type imports (5 files)
eslint/no-inline-comments trailing comments moved above their line or turned into JSDoc
eqeqeq, no-unused-vars, no-nested-ternary fixed at the source

One real architectural fix: render/css-cascade.ts and render/css-cascade-flash.ts were in an import cycle over the pct helper. It's extracted into a new dependency-free leaf module render/css-shared.ts, mirroring the existing jumbotron-shared.ts precedent.

The one scoped exception — and a repo-wide conflict worth a decision

model/columns.ts holds the East_Asian_Width WIDE_RANGES table. It carries a two-rule block disable, justified in place:

  • unicorn/numeric-separators-style would write these as 0x1_F300. They're Unicode code points and have to stay readable as the U+XXXX values the Unicode charts publish; a grouped form corresponds to nothing you can look up.

  • unicorn/number-literal-case wants uppercase hex digits (0x115F), but pnpm lint:fmt runs oxfmt, which rewrites hex digits to lowercase. Verified in isolation:

    $ printf 'export const X = [0xABCD, 0x1F300];' > hexcase.ts && oxfmt hexcase.ts
    $ cat hexcase.ts
    export const X = [0xabcd, 0x1f300];
    

    The two halves of pnpm lint disagree, so no spelling of a hex literal containing an a–f digit passes both.

I scoped the disable to this one table to honor "scope the exception narrowly," but the number-literal-case half of that conflict is repo-wide, not specific to this package. Any file that gains a hex literal with a–f digits will hit the same wall. Worth deciding separately whether to disable unicorn/number-literal-case globally in oxlint.config.mts; happy to do it in this PR if you'd rather.

2. Pre-commit — npm error EALLOWGIT

The Pre-commit workflow failed on every run of this branch:

command: (..., npm, 'install', '--allow-git=root', '-g',
          'git+file:///home/runner/.cache/pre-commit/repodwzk4_3s')
npm error code EALLOWGIT
npm error Fetching non-root packages from git has been disabled
npm error Refusing to fetch "@cursorless/talon-tools@git+file:///..."

pre-commit/action@v3.0.1 installs whatever pre-commit is latest, and pre-commit 4.6 rewrote the language: node installer:

pre-commit how it installs a node hook repo
4.5.1 local npm install, then npm pack, then npm install -g <tarball>
4.6.x npm install --allow-git=root -g git+file://<clone>

The 4.6 form is what npm's allow-git hardening refuses. This hits talon-fmt / tree-sitter-fmt, our only node-language hooks.

This is upstream's own fix, from cursorless-dev/cursorless 603c4ffdc ("Update dependency versions", #3295), which this branch predates. Taking just the workflow hunk leaves .github/workflows/pre-commit.yml byte-identical to upstream/main, so the branch rebases cleanly.

Why not bump Node instead (a dead end I ruled out)

My first attempt was to bump .nvmrc past the broken npm. That does clear EALLOWGIT — I bisected it by installing talon-tools v0.9.0 exactly the way pre-commit does:

npm result
11.9.0 (bundled with Node 24.14.0, our pin) EALLOWGIT
11.12.1 EALLOWGIT
11.13.0 (first in Node 24.16.0) ok
11.15.0 ok

But CI then failed one step later with Executable 'talon-fmt' not found — npm accepts the git spec on the newer npm yet still doesn't link the hook's bins under pre-commit 4.6. So the Node bump treats a symptom; the pre-commit version is the actual cause. That commit is dropped and .nvmrc stays at upstream's v24.14.0.

talon-tools stays at rev: v0.9.0 deliberately: later revs change formatter output and would rewrite .talon/.scm files across the repo, which is a separate change and not needed here.

Verification

  • oxlint -c oxlint.config.mts --deny-warnings . → exits 0
  • oxfmt --check . → "All matched files use the correct format", 1255 files
  • tsc in packages/command-visualizer → exits 0
  • pre-commit ran clean on both commits locally (all hooks Passed/Skipped)
  • Lint check is already green on this PR; Pre-commit re-run pending on this push

`pnpm lint` failed on the command-visualizer base branch independently of
any PR diff: 194 oxlint warnings under --deny-warnings across 24 files,
plus ~25 files oxfmt flagged as unformatted.

Every warning is fixed as a real code change, not a rule disable:

- unicorn/no-array-for-each -> for..of (10 sites)
- unicorn/prefer-string-replace-all + eslint/require-unicode-regexp in
  html.ts, tokenize.ts, derive-flashes.ts, serialize-cascade.test.ts
- unicorn/consistent-function-scoping: hoisted pct100, toPos, pctc and
  snapshot to module scope
- unicorn/custom-error-definition: ChainContinuityError now sets .name
- node/no-process-env + unicorn/import-style in fixture-root.ts
- import/no-duplicates: merged split value/type imports (5 files)
- eslint/no-inline-comments: trailing comments moved above their line or
  turned into JSDoc
- eqeqeq, no-unused-vars, no-nested-ternary: fixed at the source

One real architectural fix: css-cascade.ts and css-cascade-flash.ts were
in an import cycle over `pct`. Extracted it into a dependency-free leaf
module, render/css-shared.ts, mirroring the existing jumbotron-shared.ts.

The single exception is a two-rule block disable around the East_Asian_Width
table in model/columns.ts, justified in place:

- unicorn/numeric-separators-style would render Unicode code points as
  `0x1_F300`, which corresponds to nothing in the published Unicode charts.
- unicorn/number-literal-case wants uppercase hex digits, but `pnpm lint:fmt`
  runs oxfmt, which rewrites hex digits to lowercase. The two halves of
  `pnpm lint` disagree, so no spelling of a hex literal containing a-f
  passes both. This conflict is repo-wide, not specific to this package.

Verified: `oxlint -c oxlint.config.mts --deny-warnings .` exits 0,
`oxfmt --check .` passes 1255 files, `tsc` in the package exits 0.
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 60d8e39c-8621-4332-be2b-a4e5ba91c46e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

Comment @coderabbitai help to get the list of available commands.

The Pre-commit workflow failed on every run of this branch:

    command: (..., npm, 'install', '--allow-git=root', '-g',
              'git+file:///home/runner/.cache/pre-commit/repodwzk4_3s')
    npm error code EALLOWGIT
    npm error Fetching non-root packages from git has been disabled
    npm error Refusing to fetch "@cursorless/talon-tools@git+file:///..."

`pre-commit/action@v3.0.1` installs whatever pre-commit is latest. pre-commit
4.6 rewrote the `language: node` installer: 4.5.1 did a local `npm install`
plus `npm pack` plus `npm install -g <tarball>`, whereas 4.6 installs the hook
repo directly from its git clone with
`npm install --allow-git=root -g git+file://<clone>`. That form is what npm's
allow-git hardening refuses, and on a newer npm where the refusal goes away it
still fails to link the hook's bins ("Executable `talon-fmt` not found").

It affects talon-fmt / tree-sitter-fmt, the only node-language hooks we use.

This is upstream's own fix, from cursorless-dev/cursorless 603c4ff
("Update dependency versions", cursorless-dev#3295), which this branch predates. Taking just
the workflow hunk keeps us byte-identical to upstream/main here, so the branch
rebases cleanly.

Not a talon-tools rev-pin problem: the hook repo stays at v0.9.0 deliberately,
since later revs change formatter output and would rewrite .talon/.scm files.
@trillium
trillium force-pushed the parlay/cursorless-lint-debt2 branch from 6426ad0 to d681c3b Compare August 5, 2026 01:47
@trillium

trillium commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

Both checks this PR targets are now green: Lint ✅ and Pre-commit ✅.

Test (macos-latest, stable) still fails, but not from this diff — it is a separate pre-existing failure on the base branch:

- Downloading (300.42 MB)
Test error: Error: spawn .../.vscode-test/vscode-darwin-arm64-1.131.0/Visual Studio Code.app/Contents/MacOS/Electron ENOENT

@vscode/test-electron pulls VS Code 1.131.0 for darwin-arm64 and the extracted app has no Electron binary, so the harness never starts. Reproduced on a clean re-run of just that job. Nothing in this PR touches the VS Code extension test path — packages/command-visualizer is not loaded by it, and the other change is a workflow-only pin.

For contrast, on the earlier attempt at this work (fm/cursorless-lint-debt) the ubuntu, windows and macOS test jobs all failed; on this branch ubuntu (stable + legacy), windows and neovim all pass and only the macOS download is broken. Left alone as out of scope — happy to open a separate issue for it.

The "Test (macos-latest, stable)" job failed in the "Run VSCode tests
(Win,Mac)" step with:

  Test error: Error: spawn .../vscode-darwin-arm64-1.131.0/Visual Studio
  Code.app/Contents/MacOS/Electron ENOENT

Root cause: this is not a download or extraction failure. The archive
extracts fine -- the `code` CLI inside the very same .app successfully
installed the pokey.parse-tree dependency extension (status 0) moments
before the crash. The single missing file is the main executable.

VS Code renamed the macOS bundle executable from `Contents/MacOS/Electron`
to the product name (`Code` on Stable). A compatibility symlink preserved
the old name until it was removed, so any 1.110+ archive has only
`Contents/MacOS/Code`. @vscode/test-electron 3.0.0 hardcodes the legacy
`Electron` path, hence the ENOENT.

3.1.0 replaces the hardcoded name with a three-tier resolver: read
CFBundleExecutable from Info.plist, else the sole regular file in
Contents/MacOS/, else the legacy `Electron` name. See
microsoft/vscode-test#348 and cursorless-dev#349.

Verified locally on darwin-arm64 against the same VS Code 1.131.0 build
the runner downloads: 3.1.0 resolves to Contents/MacOS/Code (exists),
and Contents/MacOS/ contains exactly that one file -- no `Electron`.

This ports upstream cursorless-dev/cursorless commit 603c4ff
("Update dependency versions (cursorless-dev#3295)"), which made the same bump.
Only affects macOS; Linux and Windows use different code paths.
The two halves of `pnpm lint` are mutually unsatisfiable for any hex
literal containing an a-f digit:

- `pnpm lint:fmt` runs `oxfmt --check`, which rewrites hex digits to
  lowercase (`0x115F` -> `0x115f`).
- `pnpm lint:ts` runs oxlint with `style: "warn"` + `--deny-warnings`,
  and `unicorn/number-literal-case` lives in the `style` category and
  requires uppercase hex digits.

So `0x115f` fails lint:ts and `0x115F` fails lint:fmt. No spelling passes
both. This is a repo-wide property of the toolchain, not a property of any
one file, so it belongs in the shared config rather than in a
per-file disable comment.

Verified by running both halves locally: `pnpm lint:ts` and
`pnpm lint:fmt` each exit 0 with this change.

The command-visualizer Unicode width table carried a file-local disable
for this rule; that is now redundant and is narrowed to just
`unicorn/numeric-separators-style`, which remains a genuine local
override (Unicode code points must stay readable as the `U+XXXX` values
the Unicode charts publish, not as `0x1_F300`).
@trillium
trillium merged commit 6925b48 into command-visualizer Aug 5, 2026
15 of 16 checks passed
@trillium
trillium deleted the parlay/cursorless-lint-debt2 branch August 5, 2026 06:09
trillium added a commit that referenced this pull request Aug 5, 2026
* feat(command-visualizer): vendor cursorless hat-allocation algorithm

Self-contained copy of cursorless's allocateHats subgraph (chooseTokenHat,
getHatRankingContext, HatMetrics, getTokenComparator, maxByFirstDiffering)
at SHA 42452eb, plus a grapheme splitter and a tokens-in ranking wrapper.
Vendored because the algorithm is not exported from any cursorless library
entry point; see VENDOR.md. Byte-identical hat assignments, no IDE dep.

* feat(command-visualizer): animated SVG command-cascade renderer

New @cursorless/command-visualizer package: pure function from a recorded
test fixture (YAML) to a self-contained, <img>-embeddable animated SVG of a
cursorless command. Zero runtime JS in the output; one CSS --dur timeline.
Fixtures parsed with js-yaml; hat allocation via the vendored algorithm;
FlashStyle/color/shape data mirrored from cursorless with provenance notes.

* docs(command-visualizer): PR draft and integration status

* refactor(command-visualizer): import DefaultMap/CompositeKeyMap from @cursorless/lib-common

The allocate-hats vendored tree carried byte-identical copies of cursorless's
DefaultMap and CompositeKeyMap utilities. Delete both clones and re-export the
originals from @cursorless/lib-common through the common/ barrel, so every
consumer keeps its unchanged `from "../common"` import. lib-common's
CompositeKeyMap backs its store with a Map instead of a Record but exposes the
identical set/has/get/delete/clear surface the allocator uses.

@cursorless/lib-common is already a workspace dependency; no manifest change.

* feat(lib-engine): export hat-allocation and grapheme-splitter primitives

Surface the real hat-allocation building blocks and the token grapheme
splitter from the lib-engine barrel so downstream consumers can import them
instead of vendoring copies. @cursorless/command-visualizer currently carries
clones of maxByFirstDiffering, getTokenComparator and the grapheme-split regex
pinned to an old cursorless SHA; exporting the source lets it drop those.

Additive only: the allocateHats/ barrel now re-exports chooseTokenHat,
getHatRankingContext, getRankedTokens, getTokenComparator, maxByFirstDiffering,
the HatMetrics functions, and the HatCandidate/RankingContext/RankedToken/
HatMetric types alongside the existing allocateHats export; the top-level index
adds two barrel re-exports (util/allocateHats and tokenGraphemeSplitter,
covering GRAPHEME_SPLIT_REGEX, TokenGraphemeSplitter, Grapheme, UNKNOWN). No
behavior changes.

* refactor(command-visualizer): import grapheme regex + maxByFirstDiffering from lib-engine

Drop the clones that ARE safe to import from cursorless source now that
lib-engine exports them:

- GRAPHEME_SPLIT_REGEX: was defined three times (columns.ts, tokenize.ts,
  vendor/allocate-hats/splitter.ts). All now import the single source-of-truth
  from @cursorless/lib-engine, each wrapping it in a fresh RegExp so the shared
  /gu instance's lastIndex cannot leak across calls.
- maxByFirstDiffering: byte-identical to the pin and fully generic, so the
  vendored copy is deleted and vendor/chooseTokenHat.ts imports it from
  @cursorless/lib-engine.

Kept PINNED at SHA 42452eb (upstream diverged — importing would change hat
placement or fail to typecheck against the standalone's simplified types):
chooseTokenHat, HatMetrics, getHatRankingContext (forcedTokenHat /
avoidFirstLetter / isFirstLetter and the IDE-backed splitter are all post-pin),
getTokenComparator (byte-identical but typed against lib-common's full Token,
not assignable from the simplified standalone Token). Each kept file header now
states the exact reason; VENDOR.md records the full import-vs-pin split.

Adds @cursorless/lib-engine as a workspace dependency.

* refactor(command-visualizer): derive FlashStyle from @cursorless/lib-common enum

data/decorations.ts hand-maintained a string union whose five members mirrored
@cursorless/lib-common's FlashStyle enum values. Replace it with a
template-literal type `${CursorlessFlashStyle}`, so the union's members are
sourced from the enum (and track it automatically) while staying plain string
literals. This keeps every existing usage working with zero churn: DECORATION_HEX
keys, `"pendingDelete" as DecorationStyle` casts in pipeline.ts, and
`styles.has("pendingDelete")` in css-cascade.ts all still typecheck, because the
enum's values ARE those strings. No behavior change; the HEX values,
FLASH_PULSE_MS, MS_PER_STATE, HighlightStyle, and overlayPrecedence stay local
(cursorless exports none of them).

* docs(command-visualizer): re-verify fixture-mark/path dedup notes against live signatures

Step 5 investigation: the fixture-mark and fixture-path import candidates were
re-checked against the current cursorless source, not the stale prior notes.

- serializedMarksToTokenHats(marks, editor) still hard-requires a live
  TextEditor (offsetAt/getText) and returns engine TokenHat[]; parseMarks reads
  editor-less YAML into the MarkInfo render model. Not swappable — kept.
- getFixturesPath() hardcodes resources/fixtures (layout A only) and
  getCursorlessRepoRoot() throws unless CURSORLESS_REPO_ROOT is set. Our
  fixture-root keeps the dual-layout probe + $CURSORLESS_REPO default. Swapping
  would regress. Kept.
- fixture-yaml.ts already uses js-yaml's load() — the same lib/entry point
  loadFixture uses — so the YAML parsing is already deduplicated at the library
  level; no further change.

Comment-only: sharpens the provenance notes to cite the exact blocking
signatures. No code change.

* docs(command-visualizer): colors/shapes dedup decision (Option B) + status

Step 6: the hat color hexes, shape SVG d= path strings, color/shape names, and
shape adjustments have no importable TS module, so they are KEPT in
data/colors.ts / data/shapes.ts with precise provenance. Option A (a canonical
exported constants module) is rejected: the hexes live in app-vscode's
package.json as VS Code setting defaults (runtime-read, never a TS constant) and
the SVG d= strings live in resources/images/hats/*.svg (runtime-read by
VscodeHatRenderer), so any new TS constant would be a third copy; and the clean
TS constants that DO exist upstream (hatStyles.types.ts, shapeAdjustments.ts)
sit in app-vscode, whose only export is ./extension.cjs — single-sourcing them
needs promoting both to lib-common and rewiring 5 app-vscode files (the shipping
extension's hat path), the risky refactor this task scoped out.

Comments now name the exact upstream export that would be needed and record the
deferred follow-up. STATUS.md documents the full import-not-clone pass and the
Option-B rationale. Comment/doc-only; no code change.

* fix(command-visualizer): escape quotes in HTML attribute values (CodeQL XSS)

serialize-cascade.ts's esc() escaped &, <, > but not the quote characters, so a
fixture name or spoken form containing a double quote could break out of the
double-quoted data-fixture="…" / data-spoken-form="…" attributes it is
interpolated into (CodeQL: "Incomplete HTML attribute sanitization: output may
contain double quotes when it reaches an attribute definition"). esc() now also
escapes " → &quot; and ' → &cursorless-dev#39;, making it safe for both text-content and
quoted-attribute contexts. Over-escaping quotes in text (the caption / <title>)
is harmless. Verified: a fixture name of `x" onload="alert(1)"><script>…` is
fully neutralized — no attribute breakout, no tag injection.

* fix(command-visualizer): address CodeRabbit review findings R3-R10

R4 hat-allocator: oldAssignments now includes shape in styleName key
  (non-default shapes key as `${color}-${shape}` in cssStateHatStyles;
  pinned marks with shape overrides were reserving the wrong style key)

R5 css.ts: replace hardcoded hat height/voffset constants with imports
  from shapes.ts (DEFAULT_HAT_HEIGHT_EM, DEFAULT_VERTICAL_OFFSET_EM)

R6 chain.ts: guard empty states[] — throw ChainContinuityError(0) instead
  of spreading undefined

R7 chain.ts: single-step path now propagates fixtureLabel into meta.fixture

R3 fixture-yaml.ts: guard blank/whitespace input before js-yaml load()
  (js-yaml 5.x throws; {} fallback was unreachable without this guard)

R8 fixture-extract.ts: skip mark keys with no '.' separator
  (indexOf('.') == -1 produced wrong slice offsets)

R9 decorations.ts: fix stale comment '11 decoration styles' -> '7'

R10 index.ts: fix package-name header @cursorless/cascade-renderer
  -> @cursorless/command-visualizer

* refactor(lib-engine): trim allocateHats barrel to only consumed exports

@cursorless/command-visualizer imports exactly one primitive from this
sub-tree: maxByFirstDiffering (vendor/chooseTokenHat.ts, SHA 42452eb).

The broader set added in the prior export commit (chooseTokenHat,
getHatRankingContext, getRankedTokens, getTokenComparator, HatMetrics,
avoidFirstLetter, ...) are NOT consumed by command-visualizer and must
stay vendored there because they have drifted in signature since the
pinned SHA (forcedTokenHat param, avoidFirstLetter metric). Exporting
them here would invite callers to take a dependency on unstable internals.

Trimmed to: { allocateHats } (pre-existing) + { maxByFirstDiffering } (new).

* feat(lib-common): promote hat shapeAdjustments from app-vscode

Moves the shape-adjustment constants (defaultShapeAdjustments,
DEFAULT_HAT_HEIGHT_EM, DEFAULT_VERTICAL_OFFSET_EM, HatAdjustments,
IndividualHatAdjustmentMap) out of app-vscode into lib-common, alongside
the already-shared hatStyles.types, so non-VS-Code consumers (e.g.
@cursorless/command-visualizer) can import them instead of vendoring a copy.

app-vscode's original shapeAdjustments.ts becomes a backward-compatible
re-export shim, so its consumers (VscodeHatRenderer,
performPr1868ShapeUpdateInit, the hatAdjustments scripts) are unchanged.
No behavior change — values are byte-identical to the previous location.

* refactor(command-visualizer): import hat color/shape vocabulary from lib-common

Addresses PR review (colors.ts, shapes.ts flagged as duplicating cursorless).
HatColor/HAT_COLORS and HatShape/HAT_SHAPES/HAT_NON_DEFAULT_SHAPES are now
imported/re-exported from @cursorless/lib-common instead of being redefined,
along with the shape-adjustment constants (SHAPE_ADJUSTMENTS,
DEFAULT_HAT_HEIGHT_EM, DEFAULT_VERTICAL_OFFSET_EM) promoted there.

Kept local (no importable TS home — canonical source is app-vscode's
package.json VS Code config defaults / resources/*.svg, read at runtime):
  - COLOR_MATRIX / EDITOR_CHROME theme hexes
  - SHAPE_PATHS SVG 'd=' strings
Provenance comments updated to say exactly why each stays.

fixture-extract.ts: HAT_COLORS is now a readonly tuple, so the membership
cast becomes 'as readonly string[]'.

* fix(command-visualizer): escape quotes in serialize.ts esc() (CodeQL)

The duplicate esc() in serialize.ts escaped only & < > — CodeRabbit flagged
it as needing the same quote escaping already applied to serialize-cascade.ts.
Now also escapes " and ' so interpolated strings cannot break out of a
quoted HTML attribute context.

* feat(lib-common): promote hat color/shape vocabulary from app-vscode

Moves HAT_COLORS / HAT_SHAPES / HAT_NON_DEFAULT_SHAPES and the HatColor /
HatShape / HatNonDefaultShape / VscodeHatStyleName types out of app-vscode
into lib-common's hatStyles.types (which previously held only the
HatStyleName stub), so non-VS-Code consumers (e.g.
@cursorless/command-visualizer) can import the vocabulary instead of
cloning it.

app-vscode's hatStyles.types.ts becomes a backward-compatible re-export
shim, so its ~10 consumers (VscodeHats, VscodeHatRenderer, getStyleName,
keyboard/*, hatAdjustments scripts, ...) are unchanged. No behavior change.

* refactor(app-vscode): import hat vocabulary directly from lib-common

Replaces the re-export shims (hatStyles.types.ts, hats/shapeAdjustments.ts)
with direct imports from @cursorless/lib-common in every consumer, then
DELETES the shim files. No indirection layer — the vocabulary and shape
adjustments now have a single home in lib-common and every consumer imports
from it directly.

Consumers rewired: VscodeEnabledHatStyleManager, VscodeHatRenderer,
VscodeHats, getStyleName, getHatThemeColors, performPr1868ShapeUpdateInit,
scripts/hatAdjustments/{add,average}, keyboard/{TokenTypes,
KeyboardCommandsTargeted,KeyboardCommandHandler}. No behavior change.

* refactor(command-visualizer): import hat vocabulary directly from lib-common

Stops re-exporting the cursorless hat vocabulary through data/colors.ts and
data/shapes.ts. Consumers now import HatColor/HatShape/HAT_COLORS/HAT_SHAPES
and the shape-adjustment constants straight from @cursorless/lib-common.

data/colors.ts and data/shapes.ts keep ONLY the genuinely command-visualizer-
local data that has no importable TS home: COLOR_MATRIX/EDITOR_CHROME (theme
hexes from package.json config defaults) and SHAPE_PATHS (SVG d= strings from
resources/*.svg). No re-export indirection.

* refactor(command-visualizer): consolidate HTML helpers, drop unused dep

fallow surfaced esc() duplicated across 4 serializers with INCONSISTENT
escaping — svg-wrap.ts escaped no quotes (latent attribute-injection gap),
jumbotron.ts escaped " but not ', serialize/serialize-cascade escaped both.
Extracted one shared src/html.ts (esc + themeBackground + captionHtml); all
four serializers now import it, so the CodeQL-safe escaping can't drift again.
Also collapses the duplicated theme-bg + caption block (serialize-cascade
<-> svg-wrap).

Removes @cursorless/lib-node-common from dependencies — fallow flagged it as
listed-but-never-imported (only referenced in comments).

fallow dupes: 4 clone groups -> 2 (the remaining two are the pinned vendored
allocation algorithm, HatMetrics + getTokenComparator).

* test(command-visualizer): add unit tests for the deterministic core

The package shipped with ZERO tests — the verification harness the source
comments reference (verify-allocation.ts, oracle screenshots) was never
migrated from the standalone repo. Adds colocated mocha .test.ts files
(auto-discovered by packages/test-runner's unit glob, node:assert/strict,
matching the lib-engine idiom) covering the pure, deterministic functions —
especially the review-fix behaviors:

- html.test.ts: esc() escapes all 5 chars & <> " ', &-first ordering,
  attribute-breakout payload neutralized (CodeQL regression guard);
  captionHtml empty/partial/full meta + markup-injection escaping.
- fixture-yaml.test.ts: parseFixtureYaml blank/whitespace -> {} (R3),
  mapping vs scalar vs sequence, literal block scalar byte-fidelity.
- chain.test.ts: chainCascades empty-input throw w/ stepIndex 0 (R6),
  single-step fixtureLabel propagation preserving other meta (R7).

Wires @types/mocha + @types/node and types:[node,mocha] in tsconfig,
mirroring lib-engine. Typechecked against lib shims; full run needs the
networked install (same wall as the lockfile).

* chore: update pnpm-lock.yaml for command-visualizer deps (R1)

Regenerated lockfile so it matches the new dependency specifiers
(@cursorless/lib-common, @cursorless/lib-engine, js-yaml, @types/js-yaml,
@types/mocha, @types/node). Fixes the --frozen-lockfile CI install failure
CodeRabbit flagged (R1).

* refactor(command-visualizer): derive FLASH_STYLES from lib-common enum

Addresses PR review (decorations.ts: 'this can likely be an import'):
FLASH_STYLES was re-listing the FlashStyle enum members by hand — now derived
via Object.values(FlashStyle) so it tracks lib-common automatically. Order is
not significant (membership tests + per-selector CSS emission).

* feat(lib-engine): export getTokensInRange from allocateHats barrel

Exports-only change (no behavior change). command-visualizer needs the
real tokenizer-backed token extractor to find the engine token covering a
fixture mark position, so it can pin marks via forceTokenHats with matching
token identity when consuming the real allocateHats.

* refactor(command-visualizer): allocate hats via real lib-engine, drop vendored copy

Rewrites hat-allocator.ts to run cursorless's real allocateHats
(@cursorless/lib-engine) over an in-memory FakeIDE/InMemoryTextEditor
document instead of the pinned vendored copy (SHA 42452eb) under src/vendor/.
The engine tokenizes with cursorless's own tokenizer and ranks tokens by
cursor proximity.

- Deletes the entire src/vendor/allocate-hats/ tree (incl. VENDOR.md).
- Deletes word-segments.ts — the engine's own tokenizer now supplies
  word-level segmentation; the module had no other importer.
- Fixture marks are pinned via forceTokenHats: the covering engine token
  (found via getTokensInRange) is forced to the mark's exact color/shape,
  which chooseTokenHat applies first and unconditionally.
- Keeps the visualizer's own palette/penalty map (cssStateHatStyles,
  colorPenalty, styleToHat).

Rendered hat output changes vs the pinned SHA (current engine tokenizer +
ranking); byte-fidelity to the old vendored output is NOT preserved, per the
refactor directive.

* test(command-visualizer): smoke-test real-engine hat allocation

Adds hat-allocator.test.ts covering the de-vendored allocator:
- at least one hat is placed over a couple of words
- hats land on non-whitespace graphemes with valid palette colors
- a pre-attached fixture-mark hat keeps its exact color (single mark,
  multiple marks across lines) — pins verified via forceTokenHats
- allocation is deterministic for identical input
- empty document is a no-op, not a throw
- cssStateHatStyles keys pure colors and +1-penalty shape variants

This is the correctness evidence for the refactor in lieu of oracle
screenshots. 7 passing.

* refactor(command-visualizer): extract model/ scope (shared pure contract)

Move the shared, pure, HTML-free contract into model/: frame-state,
columns, overlays, timeline. Add model/geometry.ts extracting the Pos and
Range interfaces plus the orderRange helper out of the old serialize.ts so
both logic/ and render/ depend on geometry without either importing the
other. Repoint intra-model and data/ imports.

Pure relocation + import rewrite; no behavior change.

* refactor(command-visualizer): rewire logic/ imports to model/

Repoint logic/ (pipeline, fixture-extract, hat-allocator, tokenize, chain
+ their tests) at ../model/* and ../data/* for the shared contract and
constants. Pos/Range now come from ../model/geometry. No logic/ file
imports render/.

Pure import rewrite; no behavior change.

* refactor(command-visualizer): rewire render/ imports to model/

Repoint render/ (serialize, serialize-cascade, svg-wrap, jumbotron, css,
css-cascade, symbols, html) at ../model/* and ../data/*. serialize.ts now
imports Pos/Range/orderRange from ../model/geometry instead of declaring
them locally. No render/ file imports logic/.

Pure import rewrite; no behavior change.

* refactor(command-visualizer): wire public index to scopes + add ARCHITECTURE.md

Repoint the public index.ts exports at ./logic/*, ./render/*, and
./model/*. Add ARCHITECTURE.md documenting the four scopes (data/model/
logic/render), the one-directional dependency rule (logic and render never
import each other), and why columns/overlays/timeline live in model/.

Pure import rewrite + docs; no behavior change.

* test(command-visualizer): guard SHAPE_PATHS against resources/images/hats

Makes resources/images/hats/*.svg the enforced source of truth for the hat
path data. A headless/bundled renderer can't read those SVGs at runtime, so
SHAPE_PATHS keeps byte-for-byte copies — this test reads the canonical SVGs at
TEST time and asserts every shape's d= (and crosshairs' fill-rule) still
matches, so the copy can never silently drift from source. Runtime stays pure.

* refactor(command-visualizer): split css-cascade.ts under 250-line limit

Extract the flash-fade section (FADE_FRAC, DELETE/ADD/REFERENCE_FLASH_STYLES,
flashFadeKeyframes, flashFadeRules) into render/css-cascade-flash.ts. Shared
pct() formatter is exported from css-cascade.ts and reused (no duplication).
Pure extraction; rendered CSS byte-identical.

* refactor(command-visualizer): split pipeline.ts under 250-line limit

Extract two cohesive pure steps out of fixtureToCascade:
- derive-flashes.ts: step 6b char-diff synthesis of pendingDelete/justAdded
  when a fixture records no ide.flashes but the doc changed.
- derive-overlays.ts: step 7 highlights + thatMark/sourceMark -> decorations.
Both return decoration lists the caller appends; order and behavior identical.
Removed now-unused Pos/pos imports. Rendered output byte-identical.

* refactor(command-visualizer): split jumbotron.ts under 250-line limit

Split the 496-line jumbotron.ts into cohesive render/ siblings:
- jumbotron.ts keeps the markup half (commandBar/metadataBlock/dots/
  serializeJumbotron) and re-exports jumbotronCss for a stable public surface.
- jumbotron-css.ts: jumbotronCss assembler + baseCss/themedCss/carouselTrack
  section builders.
- jumbotron-css-keyframes.ts: the dot + command-pill @Keyframes builder.
- jumbotron-shared.ts: NL, frameCommands, timelinePct, commandFrameIndices —
  shared by both halves (no cycle, no duplication; fallow clean).
Rendered CSS + markup byte-identical.

* refactor(command-visualizer): legible 4-stage pipeline orchestrator

Make the render pipeline readable top-to-bottom in one place. Add a
`renderCommand` orchestrator at the package root (src/render-command.ts)
whose body shows the four stages at a glance, each delegating to a named
function:

  1. get what to render    -> parseFixture       (logic/pipeline.ts)
  2. tokenize each step     -> tokenizeStates      (logic/pipeline.ts)
  3. generate render object -> buildRenderObject   (logic/build-render-object.ts)
  4. render from object     -> serializeCascade + wrapCascadeSvg (render/)

Decompose the former 235-line fixtureToCascade into the three named stage
functions; fixtureToCascade stays exported and is now a thin composition of
them (byte-identical output). The orchestrator lives at the root, NOT in
logic/, because it is the one allowed composition point spanning both
logic/ and render/ — the folder dependency rule (logic/ ⊥ render/) is
preserved.

Shared stage types moved to logic/pipeline-types.ts to avoid a circular
pipeline <-> build-render-object edge and keep every file <=250 lines.

Public surface: add renderCommand, RenderCommandOptions, parseFixture,
tokenizeStates, buildRenderObject, ParsedFixture, TokenizedStates. All
prior exports unchanged.

Zero behavior change — rendered SVG byte-identical (verified by hash).

* refactor(command-visualizer): remove dead consts flagged in prior split

Delete four unused declarations confirmed dead by grep (zero use sites):
  - CMD_SLIDE_FRAC, LIT_HOLD_FRAC   (render/jumbotron-css.ts)
  - DELETE_FLASH_STYLES             (render/css-cascade-flash.ts; the
      "pendingDelete" literal is used directly at every call site instead)
  - aftLo local                     (render/jumbotron-css-keyframes.ts;
      aftHi is used, aftLo was computed but never read)

Zero behavior change — rendered SVG byte-identical (verified by hash).

* docs(command-visualizer): map the 4-stage pipeline in ARCHITECTURE.md

Add a "Pipeline — 4 stages" section naming each stage, its function, and
the file it lives in, so a reviewer has a top-to-bottom map:
  1 parseFixture (logic/pipeline.ts)
  2 tokenizeStates (logic/pipeline.ts)
  3 buildRenderObject (logic/build-render-object.ts)
  4 serializeCascade + wrapCascadeSvg (render/)

Document why the renderCommand orchestrator lives at the package root
(the one allowed logic+render composition point) rather than in logic/,
and list the new files (pipeline-types.ts, build-render-object.ts,
render-command.ts) in the scope inventory.

* chore(command-visualizer): conform manifest + tsconfig to meta-updater

Runs the repo's meta-updater fixer so `pnpm lint:meta` (a CI gate) passes:
- package.json: add `exports["."]`, canonical `typecheck`/`clean` scripts.
- tsconfig.json: add required `src/**/*.json` to include.
- root tsconfig.json: register command-visualizer in project `references`.
- tsconfig.base.json: add `@cursorless/command-visualizer` path mapping.
Wires the package into the workspace's project-reference + path graph like
its siblings.

* refactor(command-visualizer): rename DecorationStyle to OverlayStyleName

* refactor(command-visualizer): idiomatic casts, camelCase fn, flow-narrow hats

* docs(command-visualizer): strip dangling internal citations from comments

* docs(command-visualizer): add README, keep ARCHITECTURE, drop STATUS/PR-DRAFT

* docs(command-visualizer): track upstream reuse opportunities

Working/contributor doc: what the package can reuse from cursorless —
geometry types (Position/Range/GeneralizedRange) adoptable today with no
upstream change, plus four blocked-by-coupling items (pure flash-derivation,
editor-free serializedMarksToTokenHats, grapheme tokenizer, parameterized
fixture-path) that need a small upstream refactor+export first. Groups the
same doc-strip bucket as the pre-upstream barebones pass.

* refactor(command-visualizer): adopt lib-common Position/Range in hat-allocator and serialize

* refactor(command-visualizer): adopt lib-common GeneralizedRange family and delete local geometry

* feat(command-visualizer): thread lineNumbers option through render entry points

renderCommand now accepts lineNumbers (via CascadeRenderOptions) and
passes it into serializeCascade; serializeEditor/serializeDocument gain
the same opt-in gutter markup as the cascade path. Off by default —
output byte-identical when unset or false.

* test(command-visualizer): cover opt-in line-number gutter

Asserts data-line-numbers on the cascade root, one 1-based .cl-lineno
per line, digit-width scaling, per-frame emission, and that the default
(and lineNumbers:false) stay byte-identical with no gutter markup.

* docs(command-visualizer): record frame-state types checked, no upstream home

frame-state.ts field types already come from lib-common; container types
(Decoration/Frame/CascadeState/CascadeMeta/FrameRole/OverlayRole) have no
adoptable cursorless home. Decoration overlaps FlashDescriptor but that's
editor-coupled + flash-only + lacks role. Recorded so it isn't re-audited.

* refactor(command-visualizer): move render-model types to model/types.ts

Renames model/frame-state.ts -> model/types.ts and gives it a maintainer-
facing header: these container types (CascadeState/Frame/Decoration/FrameRole/
OverlayRole) are package-specific with no current cursorless home (field types
already come from lib-common). Isolated as a dedicated types file so cursorless
maintainers can decide whether any warrant promotion. Type-only rename +
import-path updates; zero behavior change (32 tests green, tsc clean).

* docs(command-visualizer): record hat-vocabulary consolidation landscape

The hat color/shape vocabulary existed 5x on upstream (app-vscode exported-but-
unimportable + 3 frozen legacy command schemas + a talonjs test const). This PR
moved it into lib-common's hatStyles.types.ts (was a stub) and rewired app-vscode
to consume it — first importable shared home, net duplication reduced. Records the
remaining (maintainer-side) consolidation of the non-frozen private copies.

* fix: clear pre-existing lint debt and unbreak the pre-commit workflow (#6)

* fix(command-visualizer): clear pre-existing oxlint + oxfmt debt

`pnpm lint` failed on the command-visualizer base branch independently of
any PR diff: 194 oxlint warnings under --deny-warnings across 24 files,
plus ~25 files oxfmt flagged as unformatted.

Every warning is fixed as a real code change, not a rule disable:

- unicorn/no-array-for-each -> for..of (10 sites)
- unicorn/prefer-string-replace-all + eslint/require-unicode-regexp in
  html.ts, tokenize.ts, derive-flashes.ts, serialize-cascade.test.ts
- unicorn/consistent-function-scoping: hoisted pct100, toPos, pctc and
  snapshot to module scope
- unicorn/custom-error-definition: ChainContinuityError now sets .name
- node/no-process-env + unicorn/import-style in fixture-root.ts
- import/no-duplicates: merged split value/type imports (5 files)
- eslint/no-inline-comments: trailing comments moved above their line or
  turned into JSDoc
- eqeqeq, no-unused-vars, no-nested-ternary: fixed at the source

One real architectural fix: css-cascade.ts and css-cascade-flash.ts were
in an import cycle over `pct`. Extracted it into a dependency-free leaf
module, render/css-shared.ts, mirroring the existing jumbotron-shared.ts.

The single exception is a two-rule block disable around the East_Asian_Width
table in model/columns.ts, justified in place:

- unicorn/numeric-separators-style would render Unicode code points as
  `0x1_F300`, which corresponds to nothing in the published Unicode charts.
- unicorn/number-literal-case wants uppercase hex digits, but `pnpm lint:fmt`
  runs oxfmt, which rewrites hex digits to lowercase. The two halves of
  `pnpm lint` disagree, so no spelling of a hex literal containing a-f
  passes both. This conflict is repo-wide, not specific to this package.

Verified: `oxlint -c oxlint.config.mts --deny-warnings .` exits 0,
`oxfmt --check .` passes 1255 files, `tsc` in the package exits 0.

* fix(ci): pin pre-commit to 4.5.1 so the node hooks install

The Pre-commit workflow failed on every run of this branch:

    command: (..., npm, 'install', '--allow-git=root', '-g',
              'git+file:///home/runner/.cache/pre-commit/repodwzk4_3s')
    npm error code EALLOWGIT
    npm error Fetching non-root packages from git has been disabled
    npm error Refusing to fetch "@cursorless/talon-tools@git+file:///..."

`pre-commit/action@v3.0.1` installs whatever pre-commit is latest. pre-commit
4.6 rewrote the `language: node` installer: 4.5.1 did a local `npm install`
plus `npm pack` plus `npm install -g <tarball>`, whereas 4.6 installs the hook
repo directly from its git clone with
`npm install --allow-git=root -g git+file://<clone>`. That form is what npm's
allow-git hardening refuses, and on a newer npm where the refusal goes away it
still fails to link the hook's bins ("Executable `talon-fmt` not found").

It affects talon-fmt / tree-sitter-fmt, the only node-language hooks we use.

This is upstream's own fix, from cursorless-dev/cursorless 603c4ff
("Update dependency versions", cursorless-dev#3295), which this branch predates. Taking just
the workflow hunk keeps us byte-identical to upstream/main here, so the branch
rebases cleanly.

Not a talon-tools rev-pin problem: the hook repo stays at v0.9.0 deliberately,
since later revs change formatter output and would rewrite .talon/.scm files.

* fix(ci): bump @vscode/test-electron to ^3.1.0 to fix macOS test run

The "Test (macos-latest, stable)" job failed in the "Run VSCode tests
(Win,Mac)" step with:

  Test error: Error: spawn .../vscode-darwin-arm64-1.131.0/Visual Studio
  Code.app/Contents/MacOS/Electron ENOENT

Root cause: this is not a download or extraction failure. The archive
extracts fine -- the `code` CLI inside the very same .app successfully
installed the pokey.parse-tree dependency extension (status 0) moments
before the crash. The single missing file is the main executable.

VS Code renamed the macOS bundle executable from `Contents/MacOS/Electron`
to the product name (`Code` on Stable). A compatibility symlink preserved
the old name until it was removed, so any 1.110+ archive has only
`Contents/MacOS/Code`. @vscode/test-electron 3.0.0 hardcodes the legacy
`Electron` path, hence the ENOENT.

3.1.0 replaces the hardcoded name with a three-tier resolver: read
CFBundleExecutable from Info.plist, else the sole regular file in
Contents/MacOS/, else the legacy `Electron` name. See
microsoft/vscode-test#348 and cursorless-dev#349.

Verified locally on darwin-arm64 against the same VS Code 1.131.0 build
the runner downloads: 3.1.0 resolves to Contents/MacOS/Code (exists),
and Contents/MacOS/ contains exactly that one file -- no `Electron`.

This ports upstream cursorless-dev/cursorless commit 603c4ff
("Update dependency versions (cursorless-dev#3295)"), which made the same bump.
Only affects macOS; Linux and Windows use different code paths.

* fix(lint): disable unicorn/number-literal-case repo-wide

The two halves of `pnpm lint` are mutually unsatisfiable for any hex
literal containing an a-f digit:

- `pnpm lint:fmt` runs `oxfmt --check`, which rewrites hex digits to
  lowercase (`0x115F` -> `0x115f`).
- `pnpm lint:ts` runs oxlint with `style: "warn"` + `--deny-warnings`,
  and `unicorn/number-literal-case` lives in the `style` category and
  requires uppercase hex digits.

So `0x115f` fails lint:ts and `0x115F` fails lint:fmt. No spelling passes
both. This is a repo-wide property of the toolchain, not a property of any
one file, so it belongs in the shared config rather than in a
per-file disable comment.

Verified by running both halves locally: `pnpm lint:ts` and
`pnpm lint:fmt` each exit 0 with this change.

The command-visualizer Unicode width table carried a file-local disable
for this rule; that is now redundant and is narrowed to just
`unicorn/numeric-separators-style`, which remains a genuine local
override (Unicode code points must stay readable as the `U+XXXX` values
the Unicode charts publish, not as `0x1_F300`).
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