Command visualizer - #3
Merged
Merged
Conversation
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.
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.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a new ChangesCommand Visualizer Package
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Fixture as Fixture YAML
participant Pipeline as pipeline.ts
participant BuildRenderObject as build-render-object.ts
participant HatAllocator as hat-allocator.ts
participant SerializeCascade as serialize-cascade.ts
participant SvgWrap as svg-wrap.ts
Fixture->>Pipeline: parseFixture(src, fixtureRel)
Pipeline->>Pipeline: tokenizeStates(parsed)
Pipeline->>HatAllocator: buildLines / allocateHats
HatAllocator-->>Pipeline: tokenized lines with hats
Pipeline->>BuildRenderObject: buildRenderObject(parsed, tokenized)
BuildRenderObject-->>Pipeline: CascadeState
Pipeline-->>SerializeCascade: serializeCascade(state)
SerializeCascade-->>SvgWrap: wrapCascadeSvg(state, inner)
SvgWrap-->>Fixture: standalone animated SVG string
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
trillium
commented
Jul 7, 2026
trillium
commented
Jul 7, 2026
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
trillium
commented
Jul 7, 2026
|
|
||
| export type Theme = "dark" | "light"; | ||
|
|
||
| export const COLOR_MATRIX: Record<Theme, Record<HatColor, string>> = { |
Owner
Author
There was a problem hiding this comment.
we ideally can pull this in from elsewhere too, do not need to define ourselves
trillium
commented
Jul 7, 2026
trillium
commented
Jul 7, 2026
| // flashRanges(...) → await sleep(getPendingEditDecorationTime()) → clear | ||
| // A flash is a FIXED 100ms pulse, decoupled from the readability state-hold | ||
| // cadence (SPEC-v2 §4.2, B2/B6). Both the delete (pendingDelete) and insert | ||
| // (justAdded) beats are pinned to this. verify:flash-timing is the oracle. |
Owner
Author
There was a problem hiding this comment.
This may be defined already in cursorless, please check
trillium
commented
Jul 7, 2026
| // changing MS_PER_STATE rescales the state-hold but never the 100ms flash. | ||
| export const MS_PER_STATE = 1000; | ||
|
|
||
| export const ALL_DECORATION_STYLES: DecorationStyle[] = [ |
Owner
Author
There was a problem hiding this comment.
Also this, likely defined already
trillium
commented
Jul 7, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (7)
packages/command-visualizer/src/tokenize.ts (1)
43-61: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueGap handling splits by UTF-16 code unit, not codepoint.
emitGapAsTokensindexestext[i]/text.slice(i, j)per UTF-16 unit. For unmatched gap characters outside the grapheme regex's categories (e.g. format/control codepoints from the astral plane), a single codepoint spanning a surrogate pair would be split into two separate single-unit tokens, breaking the grapheme. The comment already flags this as an approximation ("in practice gaps are whitespace"), so this is a narrow edge case rather than a blocking bug.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/tokenize.ts` around lines 43 - 61, The gap tokenization in emitGapAsTokens currently walks text by UTF-16 code unit, which can split a single astral-plane codepoint into two tokens in the non-whitespace branch. Update tokenize.ts so emitGapAsTokens iterates over codepoints or grapheme-safe boundaries when emitting unmatched gap characters, while preserving the current coalescing behavior for whitespace runs. Use the existing text/tokens logic in emitGapAsTokens as the place to make the boundary handling safe.packages/command-visualizer/src/serialize.ts (2)
59-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant duplicate branch in
charToCol.The
<and===cases both returnc.col; can be collapsed, as already done in theserialize-cascade.tsversion of this function.🧹 Proposed simplification
function charToCol(cols: Column[], charIndex: number): number { for (const c of cols) { - if (charIndex < c.charIndex) { - return c.col; - } - // last char unit of this cell: account for multi-code-unit graphemes - if (charIndex === c.charIndex) { + if (charIndex <= c.charIndex) { return c.col; } } return lineWidth(cols); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/serialize.ts` around lines 59 - 70, The charToCol function contains a redundant split between the charIndex < c.charIndex and charIndex === c.charIndex branches, since both return the same column value. Simplify the loop in charToCol by combining these conditions into a single check that returns c.col, matching the existing approach used in serialize-cascade.ts, while leaving the fallback to lineWidth(cols) unchanged.
25-27: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
esc()doesn't escape double quotes — safe here, but this exact helper is duplicated and used in attribute context elsewhere.Not exploitable in this file since
esc()is only used in text-content positions here, but the same escaping logic is duplicated inserialize-cascade.tswhere it is used to populate double-quoted HTML attributes (data-fixture="${esc(...)}"), which is unsafe. See the corresponding comment onserialize-cascade.ts(Lines 17-19, 160, 163) for the concrete fix; consider extracting one sharedesc()/escAttr()utility so the fix applies everywhere at once.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/serialize.ts` around lines 25 - 27, The escaping helper is duplicated and currently only handles text-content escaping, but the same logic is also reused for double-quoted HTML attributes elsewhere. Update the shared escaping approach used by esc() and the corresponding helper in serialize-cascade.ts so attribute values are safely escaped too, preferably by extracting a common esc/escAttr utility and switching both serialize.ts and serialize-cascade.ts to use it.Source: Linters/SAST tools
packages/command-visualizer/src/fixture-root.ts (1)
46-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate layout-probing logic between
hatsRoot()andfixtureRoot().Both functions repeat the same two-layout existence-check pattern differing only in path segments/labels. Extract a shared helper.
♻️ Proposed refactor
+function probeLayout(root: string, what: string, layoutA: string, layoutB: string): string { + const a = join(root, ...layoutA.split("/")); + if (existsSync(a)) return a; + const b = join(root, ...layoutB.split("/")); + if (existsSync(b)) return b; + throw new Error( + `cursorless ${what} not found in "${root}".\nExpected one of:\n ${a}\n ${b}`, + ); +} + export function hatsRoot(): string { - const root = cursorlessRepoRoot(); - const layoutA = join(root, "resources", "images", "hats"); - if (existsSync(layoutA)) { - return layoutA; - } - const layoutB = join(root, "images", "hats"); - if (existsSync(layoutB)) { - return layoutB; - } - throw new Error( - `cursorless hat SVGs not found in "${root}".\nExpected one of:\n ${layoutA}\n ${layoutB}`, - ); + return probeLayout(cursorlessRepoRoot(), "hat SVGs", "resources/images/hats", "images/hats"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/fixture-root.ts` around lines 46 - 82, Both hatsRoot() and fixtureRoot() duplicate the same two-layout probing pattern, so extract the shared existence-check logic into a small helper and have both functions delegate to it. Keep the layout-specific path segments and error labels in hatsRoot() and fixtureRoot(), but centralize the repeated existsSync/join/throw behavior in a reusable function so the repo-layout detection stays consistent and easier to maintain.packages/command-visualizer/src/pipeline.ts (1)
209-227: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUnvalidated
stylecast from fixture YAML.
String(fo.style) as DecorationStyle(and the analogous cast for highlights at Line 264) blindly asserts the type without checking membership in theDecorationStyleunion — unlikerange, which is validated viatoGeneralizedRangereturningnullon failure. A malformed/garbled fixture would silently produce an invalid style flowing into decorations, and downstreamDECORATION_HEX[style]lookups (css-cascade.ts) would just render nothing rather than fail loudly.🛡️ Proposed guard
const style = String(fo.style) as DecorationStyle; + if (!isKnownDecorationStyle(style)) { + continue; + } const range = toGeneralizedRange(asObj(fo.range) ?? {});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/pipeline.ts` around lines 209 - 227, The `style` value parsed from fixture YAML is being force-cast in `pipeline.ts` without validation, so malformed input can slip through and later break decoration rendering. Add a guard in the flashes loop (and the analogous highlights path) that checks `fo.style` against the allowed `DecorationStyle` values before pushing into `afterFrame.decorations` or `duringFlashes`; if it is invalid, skip or reject it the same way `toGeneralizedRange` returns null for bad ranges. Use the existing decoration-building flow around `flashRidesAfter`, `afterFrame`, and `duringFlashes` to keep invalid styles from reaching downstream lookups like `DECORATION_HEX`.packages/command-visualizer/src/vendor/allocate-hats/index.ts (1)
117-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a stricter key type for
HAT_COLOR_PENALTIES.Typed as
Record<string, number>, so a typo'd or missing color key in this map wouldn't be caught at compile time. Keying it toHAT_COLORSwould let TypeScript enforce completeness.♻️ Proposed tightening
-export const HAT_COLOR_PENALTIES: Record<string, number> = { +export const HAT_COLOR_PENALTIES: Record<(typeof HAT_COLORS)[number], number> = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/vendor/allocate-hats/index.ts` around lines 117 - 127, HAT_COLOR_PENALTIES is too loosely typed as Record<string, number>, so typos or missing hat colors won’t be caught by TypeScript. Tighten the key type in the allocate-hats module by deriving it from HAT_COLORS and update HAT_COLOR_PENALTIES to use that exact color union so the map is checked for completeness and invalid keys are rejected at compile time.packages/command-visualizer/src/vendor/allocate-hats/vendor/chooseTokenHat.ts (1)
21-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocstring's rank-order assumption no longer matches this port's convention.
Line 25-27 states this function "assumes that all tokens with a lower rank than the given token have already been assigned hats." That describes the upstream convention where lower numeric rank = higher priority (processed first). In this port,
rank.tsinverts the scheme (rank = -index, so a higher numeric rank is "more important"), andindex.tsprocessessortedRankedin descending order (best/highest rank first) — the opposite of what this comment literally describes. The implementation is functionally correct (confirmed viaindex.ts's own comment "Process tokens in descending rank order (best tokens first)"), but this stale docstring can mislead future maintainers reasoning about the algorithm's invariants. Since this file is vendored "IMPORT REWRITES ONLY" (per header) and presumably kept byte-similar to upstream for future re-vendoring diffs, consider adding a clarifying note in the calling code (index.ts) near thesortedRankedloop instead of editing this vendored file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/vendor/allocate-hats/vendor/chooseTokenHat.ts` around lines 21 - 56, The rank-order assumption in chooseTokenHat.ts is stale for this port and can mislead maintainers about processing order. Keep the vendored docstring unchanged, and instead add a clarifying note in index.ts near the sortedRanked loop and related rank.ts logic that this port processes tokens in descending rank order (best/highest rank first) because rank is inverted from index. Reference chooseTokenHat and sortedRanked so future readers understand the invariant without changing the vendored file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/command-visualizer/package.json`:
- Around line 22-29: The lockfile is out of sync with the updated dependencies
in package.json, so CI with frozen installs fails. Regenerate the workspace
lockfile by running pnpm install at the repo root after the dependency changes,
and commit the updated pnpm-lock.yaml so it reflects the new
`@cursorless/lib-common`, `@cursorless/lib-node-common`, js-yaml, and `@types/js-yaml`
specifiers.
In `@packages/command-visualizer/src/chain.ts`:
- Around line 74-76: The early return in the chain-building logic leaves a
single-state result with its original meta.fixture, which makes fixtureLabel
handling inconsistent with multi-step chains. Update the chain function to apply
the same final meta normalization for the states.length === 1 case as the
multi-step path, so the returned result always uses fixtureLabel and preserves
the expected meta shape consistently. Use the chain function and the
states.length === 1 branch as the fix point, and keep the existing
spokenForm/action fields intact when rewriting meta.
- Around line 70-135: Add an explicit empty-input guard at the start of
chainCascades so an empty states array cannot fall through to the final object
spread. In chainCascades, before the existing length===1 shortcut and merge
loop, reject states.length===0 with a clear error (or otherwise handle it
intentionally) so the function never returns a CascadeState missing fields like
theme/tabSize. Keep the fix localized to chainCascades and ensure the returned
value remains a valid CascadeState for serializeCascade/css-cascade.ts.
In `@packages/command-visualizer/src/css.ts`:
- Around line 4-5: The CSS values for hat sizing/offset are hardcoded and can
drift from the source constants used by the generated styles. Update the CSS
generation in css.ts to import and interpolate DEFAULT_HAT_HEIGHT_EM and
DEFAULT_VERTICAL_OFFSET_EM from shapes.ts (or the existing shapes data source)
so --hat-height and --hat-base-voffset are derived from the same single source
of truth. Keep the logic aligned with the other generated values in the
stylesheet so changes in HAT_SHAPES/SHAPE_ADJUSTMENTS stay reflected
automatically.
In `@packages/command-visualizer/src/data/decorations.ts`:
- Around line 23-24: Update the comment in the DECORATION_HEX definition so it
matches the actual DecorationStyle set: there are 7 styles total (5 FlashStyle
plus 2 HighlightStyle), and the Record<DecorationStyle, string> already enforces
that exact key set. Remove the “All 11 decoration styles” wording and revise the
parenthetical about scope-pair styles to reflect the 7 defined entries so the
comment accurately describes the data in decorations.ts.
In `@packages/command-visualizer/src/fixture-extract.ts`:
- Around line 54-64: In fixture-extract.ts, the mark parsing in the loop over
Object.entries(marksObj) assumes every key contains a "." and can silently
misparse malformed keys. Update the parsing around the dot, colorRaw, and
grapheme extraction to explicitly handle key.indexOf(".") === -1 by skipping or
rejecting that entry before slicing, so only well-formed keys reach the
color/grapheme logic in the extraction flow.
In `@packages/command-visualizer/src/fixture-yaml.ts`:
- Around line 33-42: The parseFixtureYaml helper currently calls load() before
checking for empty input, so blank or whitespace-only fixture YAML can still
throw and bypass the {} fallback. Update parseFixtureYaml to guard against blank
src before invoking js-yaml load, and keep the existing object/array checks so
callers still get a plain object result for non-empty mappings.
In `@packages/command-visualizer/src/hat-allocator.ts`:
- Around line 124-138: The hat allocator is reserving only the hat color for
pinned marks, which can miss distinct styles when a mark uses a shape override.
Update the `oldAssignments` creation in `hat-allocator.ts` so the reserved
`styleName` matches the full key used by `cssStateHatStyles()`, including both
the hat color and any shape override from `g.hat`. Keep the existing pinning
logic in `segments.forEach`/`g.hat` intact, but ensure the style reservation
uses the same `${color}-${shape}` convention as the style map.
In `@packages/command-visualizer/src/index.ts`:
- Around line 1-3: The header comment in the public entry point uses the wrong
package name. Update the comment at the top of the index.ts surface so it refers
to `@cursorless/command-visualizer` instead of `@cursorless/cascade-renderer`,
keeping the rest of the public surface description aligned with the actual
package.
In `@packages/command-visualizer/src/serialize-cascade.ts`:
- Around line 17-19: The esc() helper used by serialize-cascade and the
duplicate esc() in serialize.ts only escape &, <, and >, which is unsafe for
HTML attribute values like data-fixture and data-spoken-form. Update esc() to
also escape double quotes so attribute contents cannot break out of the quoted
context, and make sure both serializers use the same fix or a shared escaping
utility to keep behavior consistent.
---
Nitpick comments:
In `@packages/command-visualizer/src/fixture-root.ts`:
- Around line 46-82: Both hatsRoot() and fixtureRoot() duplicate the same
two-layout probing pattern, so extract the shared existence-check logic into a
small helper and have both functions delegate to it. Keep the layout-specific
path segments and error labels in hatsRoot() and fixtureRoot(), but centralize
the repeated existsSync/join/throw behavior in a reusable function so the
repo-layout detection stays consistent and easier to maintain.
In `@packages/command-visualizer/src/pipeline.ts`:
- Around line 209-227: The `style` value parsed from fixture YAML is being
force-cast in `pipeline.ts` without validation, so malformed input can slip
through and later break decoration rendering. Add a guard in the flashes loop
(and the analogous highlights path) that checks `fo.style` against the allowed
`DecorationStyle` values before pushing into `afterFrame.decorations` or
`duringFlashes`; if it is invalid, skip or reject it the same way
`toGeneralizedRange` returns null for bad ranges. Use the existing
decoration-building flow around `flashRidesAfter`, `afterFrame`, and
`duringFlashes` to keep invalid styles from reaching downstream lookups like
`DECORATION_HEX`.
In `@packages/command-visualizer/src/serialize.ts`:
- Around line 59-70: The charToCol function contains a redundant split between
the charIndex < c.charIndex and charIndex === c.charIndex branches, since both
return the same column value. Simplify the loop in charToCol by combining these
conditions into a single check that returns c.col, matching the existing
approach used in serialize-cascade.ts, while leaving the fallback to
lineWidth(cols) unchanged.
- Around line 25-27: The escaping helper is duplicated and currently only
handles text-content escaping, but the same logic is also reused for
double-quoted HTML attributes elsewhere. Update the shared escaping approach
used by esc() and the corresponding helper in serialize-cascade.ts so attribute
values are safely escaped too, preferably by extracting a common esc/escAttr
utility and switching both serialize.ts and serialize-cascade.ts to use it.
In `@packages/command-visualizer/src/tokenize.ts`:
- Around line 43-61: The gap tokenization in emitGapAsTokens currently walks
text by UTF-16 code unit, which can split a single astral-plane codepoint into
two tokens in the non-whitespace branch. Update tokenize.ts so emitGapAsTokens
iterates over codepoints or grapheme-safe boundaries when emitting unmatched gap
characters, while preserving the current coalescing behavior for whitespace
runs. Use the existing text/tokens logic in emitGapAsTokens as the place to make
the boundary handling safe.
In `@packages/command-visualizer/src/vendor/allocate-hats/index.ts`:
- Around line 117-127: HAT_COLOR_PENALTIES is too loosely typed as
Record<string, number>, so typos or missing hat colors won’t be caught by
TypeScript. Tighten the key type in the allocate-hats module by deriving it from
HAT_COLORS and update HAT_COLOR_PENALTIES to use that exact color union so the
map is checked for completeness and invalid keys are rejected at compile time.
In
`@packages/command-visualizer/src/vendor/allocate-hats/vendor/chooseTokenHat.ts`:
- Around line 21-56: The rank-order assumption in chooseTokenHat.ts is stale for
this port and can mislead maintainers about processing order. Keep the vendored
docstring unchanged, and instead add a clarifying note in index.ts near the
sortedRanked loop and related rank.ts logic that this port processes tokens in
descending rank order (best/highest rank first) because rank is inverted from
index. Reference chooseTokenHat and sortedRanked so future readers understand
the invariant without changing the vendored file.
🪄 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: 9141146c-19c1-434c-bded-db40ec43c033
📒 Files selected for processing (40)
packages/command-visualizer/PR-DRAFT.mdpackages/command-visualizer/STATUS.mdpackages/command-visualizer/package.jsonpackages/command-visualizer/src/chain.tspackages/command-visualizer/src/columns.tspackages/command-visualizer/src/css-cascade.tspackages/command-visualizer/src/css.tspackages/command-visualizer/src/data/colors.tspackages/command-visualizer/src/data/decorations.tspackages/command-visualizer/src/data/shapes.tspackages/command-visualizer/src/fixture-extract.tspackages/command-visualizer/src/fixture-root.tspackages/command-visualizer/src/fixture-yaml.tspackages/command-visualizer/src/frame-state.tspackages/command-visualizer/src/hat-allocator.tspackages/command-visualizer/src/index.tspackages/command-visualizer/src/jumbotron.tspackages/command-visualizer/src/overlays.tspackages/command-visualizer/src/pipeline.tspackages/command-visualizer/src/serialize-cascade.tspackages/command-visualizer/src/serialize.tspackages/command-visualizer/src/svg-wrap.tspackages/command-visualizer/src/symbols.tspackages/command-visualizer/src/timeline.tspackages/command-visualizer/src/tokenize.tspackages/command-visualizer/src/vendor/allocate-hats/VENDOR.mdpackages/command-visualizer/src/vendor/allocate-hats/common/CompositeKeyMap.tspackages/command-visualizer/src/vendor/allocate-hats/common/DefaultMap.tspackages/command-visualizer/src/vendor/allocate-hats/common/index.tspackages/command-visualizer/src/vendor/allocate-hats/common/types.tspackages/command-visualizer/src/vendor/allocate-hats/index.tspackages/command-visualizer/src/vendor/allocate-hats/rank.tspackages/command-visualizer/src/vendor/allocate-hats/splitter.tspackages/command-visualizer/src/vendor/allocate-hats/vendor/HatMetrics.tspackages/command-visualizer/src/vendor/allocate-hats/vendor/chooseTokenHat.tspackages/command-visualizer/src/vendor/allocate-hats/vendor/getHatRankingContext.tspackages/command-visualizer/src/vendor/allocate-hats/vendor/getTokenComparator.tspackages/command-visualizer/src/vendor/allocate-hats/vendor/maxByFirstDiffering.tspackages/command-visualizer/src/word-segments.tspackages/command-visualizer/tsconfig.json
Comment on lines
+22
to
+29
| "dependencies": { | ||
| "@cursorless/lib-common": "workspace:*", | ||
| "@cursorless/lib-node-common": "workspace:*", | ||
| "js-yaml": "^5.2.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/js-yaml": "^4.0.9" | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Lockfile out of sync — CI install fails.
Pipeline failures across ubuntu/windows/lint jobs all report pnpm-lock.yaml doesn't match the new dependency specifiers (js-yaml@^5.2.1, @types/js-yaml@^4.0.9, @cursorless/lib-common@workspace:*, @cursorless/lib-node-common@workspace:*), causing --frozen-lockfile install to fail.
Run pnpm install at the repo root and commit the updated lockfile.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/command-visualizer/package.json` around lines 22 - 29, The lockfile
is out of sync with the updated dependencies in package.json, so CI with frozen
installs fails. Regenerate the workspace lockfile by running pnpm install at the
repo root after the dependency changes, and commit the updated pnpm-lock.yaml so
it reflects the new `@cursorless/lib-common`, `@cursorless/lib-node-common`,
js-yaml, and `@types/js-yaml` specifiers.
Source: Pipeline failures
…@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.
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.
…ring 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.
…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).
…inst 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.
…tatus 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.
…QL 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 " → " 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.
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
@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).
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.
…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[]'.
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.
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.
Owner
Author
Review findings addressedWent through every review comment (submitted + the 6 drafts still in a pending review). Grouped by resolution. ✅ Deduped — now imported from cursorless sourcePromoted the shared vocabulary from
✅ Kept local — no importable TS home (canonical source is config/SVG, read at runtime)These have no compile-time constant anywhere to import; a headless renderer must mirror them. Provenance comments sharpened to point at the exact source:
✅ CodeRabbit / CodeQL fixes
⏳ Requires a networked machine (can't validate offline)
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/command-visualizer/src/serialize-cascade.ts (1)
25-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
esc()into a shared module to eliminate duplication.The
esc()function is identical inserialize.ts(lines 29-36). Since this is a security-critical escaping utility, a single source of truth prevents silent divergence if one copy is updated without the other.♻️ Proposed refactor
Create a shared module (e.g.
src/escape.ts):/** HTML escaper safe for text content AND quoted-attribute contexts. */ export function esc(s: string): string { return s .replace(/&/g, "&") .replace(/</g, "<") .replace(/>/g, ">") .replace(/"/g, """) .replace(/'/g, "&`#39`;"); }Then in both
serialize-cascade.tsandserialize.ts:+import { esc } from "./escape"; - -// HTML escaper safe for BOTH text content AND quoted-attribute contexts. -// ... -function esc(s: string): string { - return s - .replace(/&/g, "&") - .replace(/</g, "<") - .replace(/>/g, ">") - .replace(/"/g, """) - .replace(/'/g, "&`#39`;"); -}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/serialize-cascade.ts` around lines 25 - 31, The duplicated esc() implementation in serialize-cascade and serialize should be moved to a shared escape module so there is one source of truth for HTML escaping. Create a shared esc() utility and update both serializeCascade and serialize to import and use it, keeping the same escaping behavior for text and quoted attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/command-visualizer/src/serialize-cascade.ts`:
- Around line 25-31: The duplicated esc() implementation in serialize-cascade
and serialize should be moved to a shared escape module so there is one source
of truth for HTML escaping. Create a shared esc() utility and update both
serializeCascade and serialize to import and use it, keeping the same escaping
behavior for text and quoted attributes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34709df9-35c1-4d5f-9b08-354037b990cb
📒 Files selected for processing (30)
packages/app-vscode/src/ide/vscode/hatStyles.types.tspackages/app-vscode/src/ide/vscode/hats/shapeAdjustments.tspackages/command-visualizer/STATUS.mdpackages/command-visualizer/package.jsonpackages/command-visualizer/src/chain.tspackages/command-visualizer/src/columns.tspackages/command-visualizer/src/css.tspackages/command-visualizer/src/data/colors.tspackages/command-visualizer/src/data/decorations.tspackages/command-visualizer/src/data/shapes.tspackages/command-visualizer/src/fixture-extract.tspackages/command-visualizer/src/fixture-root.tspackages/command-visualizer/src/fixture-yaml.tspackages/command-visualizer/src/hat-allocator.tspackages/command-visualizer/src/index.tspackages/command-visualizer/src/serialize-cascade.tspackages/command-visualizer/src/serialize.tspackages/command-visualizer/src/tokenize.tspackages/command-visualizer/src/vendor/allocate-hats/VENDOR.mdpackages/command-visualizer/src/vendor/allocate-hats/common/index.tspackages/command-visualizer/src/vendor/allocate-hats/splitter.tspackages/command-visualizer/src/vendor/allocate-hats/vendor/HatMetrics.tspackages/command-visualizer/src/vendor/allocate-hats/vendor/chooseTokenHat.tspackages/command-visualizer/src/vendor/allocate-hats/vendor/getHatRankingContext.tspackages/command-visualizer/src/vendor/allocate-hats/vendor/getTokenComparator.tspackages/lib-common/src/ide/types/hatStyles.types.tspackages/lib-common/src/ide/types/shapeAdjustments.tspackages/lib-common/src/index.tspackages/lib-engine/src/index.tspackages/lib-engine/src/util/allocateHats/index.ts
✅ Files skipped from review due to trivial changes (1)
- packages/command-visualizer/src/vendor/allocate-hats/VENDOR.md
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/command-visualizer/package.json
- packages/command-visualizer/src/vendor/allocate-hats/common/index.ts
- packages/command-visualizer/src/index.ts
- packages/command-visualizer/src/data/decorations.ts
- packages/command-visualizer/src/fixture-yaml.ts
- packages/command-visualizer/src/vendor/allocate-hats/vendor/getTokenComparator.ts
- packages/command-visualizer/src/columns.ts
- packages/command-visualizer/src/fixture-root.ts
- packages/command-visualizer/src/vendor/allocate-hats/vendor/getHatRankingContext.ts
- packages/command-visualizer/src/vendor/allocate-hats/vendor/HatMetrics.ts
- packages/command-visualizer/src/hat-allocator.ts
- packages/command-visualizer/src/fixture-extract.ts
- packages/command-visualizer/src/chain.ts
- packages/command-visualizer/src/css.ts
- packages/command-visualizer/src/vendor/allocate-hats/vendor/chooseTokenHat.ts
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.
…-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.
… 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.
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.
…act) 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.
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.
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.
…TECTURE.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.
…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.
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.
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.
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.
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).
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).
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.
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.
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.
…allocator and serialize
…y and delete local geometry
…try 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.
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.
…am 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.
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).
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.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/command-visualizer/src/render/serialize-cascade.test.ts (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
uflag to all regex literals — eslintrequire-unicode-regexpwarnings.Five regex literals are missing the
u(unicode) flag, triggering eslint warnings. Adding it is a one-character fix per pattern and aligns with the project's lint configuration.♻️ Proposed fix
- return [...html.matchAll(/<span class="cl-lineno"[^>]*>([^<]*)<\/span>/g)].map( + return [...html.matchAll(/<span class="cl-lineno"[^>]*>([^<]*)<\/span>/gu)].map(- assert.match(html, /--gutter-digits:1;/); + assert.match(html, /--gutter-digits:1;/u);- assert.equal((html.match(/class="cl-line"/g) ?? []).length, 3); + assert.equal((html.match(/class="cl-line"/gu) ?? []).length, 3);- assert.match(html, /--gutter-digits:2;/); + assert.match(html, /--gutter-digits:2;/u);- assert.equal((html.match(/class="frame"/g) ?? []).length, 2); + assert.equal((html.match(/class="frame"/gu) ?? []).length, 2);Also applies to: 44-44, 49-49, 72-72, 92-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/render/serialize-cascade.test.ts` at line 27, Five regex literals in serialize-cascade.test.ts are missing the unicode flag and are triggering require-unicode-regexp warnings. Update each regex used in the serialize-cascade test helpers to include the u flag, including the pattern in the html.matchAll call and the other referenced regex literals in this test file. Keep the existing patterns and only add unicode support so the lint warnings are cleared.Source: Linters/SAST tools
packages/command-visualizer/src/render/css-cascade.ts (1)
131-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider single-sourcing editor theme color values.
cascadeThemeBridge()hardcodes the same--editor-bg/--editor-fg/--editor-sel/--editor-caretvalues thatthemeVars()incss.tsgenerates. The comment acknowledges the mirroring, but a future update to one set without the other will cause the cascade box to visually drift from the editor surface. Consider exporting the editor color constants from a single location (e.g.,data/colors.tsordata/decorations.ts) and referencing them in boththemeVarsandcascadeThemeBridge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/render/css-cascade.ts` around lines 131 - 134, The editor theme color values are duplicated between cascadeThemeBridge() and themeVars(), so update the implementation to single-source these values from one shared location instead of hardcoding them twice. Move or export the editor color constants from a common module such as data/colors.ts or data/decorations.ts, then have both css.ts’s themeVars() and css-cascade.ts’s cascadeThemeBridge() reference the same symbols so the dark/light editor styles stay in sync.packages/command-visualizer/src/render/jumbotron.ts (1)
80-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCompute
timelineOf(state.frames)once instead of repeating in each sub-function.
commandBar(line 80),metadataBlock(line 102), anddots(line 133) each independently calltimelineOf(state.frames)to read.totalMs. SinceserializeJumbotronorchestrates all three, computing the timeline once and passing it down (or readingtotalMsfrom a shared variable) eliminates redundant work and ensures consistency if the timeline logic ever changes.♻️ Suggested refactor
export function serializeJumbotron( state: CascadeState, inner: string, opts: { barWidthPx?: number } = {}, ): string { + const tl = timelineOf(state.frames); return ( `<div class="visualizer-wrapper visualizer-jumbotron" data-theme="${state.theme}">` + NL + `<div class="jumbotron-container">${NL}${inner}${NL}</div>` + NL + - commandBar(state, opts.barWidthPx) + + commandBar(state, opts.barWidthPx, tl.totalMs) + NL + - metadataBlock(state) + + metadataBlock(state, tl.totalMs) + NL + - dots(state) + + dots(state, tl.totalMs) + NL + `</div>` ); }Then update each sub-function to accept
totalMs: numberinstead of callingtimelineOfinternally.Also applies to: 102-102, 133-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/command-visualizer/src/render/jumbotron.ts` at line 80, serializeJumbotron and its helpers recompute the timeline repeatedly; compute timelineOf(state.frames) once, store the shared totalMs, and pass that value into commandBar, metadataBlock, and dots instead of having each function call timelineOf independently. Update the relevant helper signatures and their call sites so they read the shared totalMs value for the style/metadata/dots rendering paths.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/command-visualizer/README.md`:
- Around line 24-26: The fenced diagram in README is missing a language label
and is triggering markdownlint MD040. Update the fence around the diagram to be
labeled as text so the documentation remains lint-clean, and make the change in
the README block containing the pre/bump/post diagram.
In `@packages/command-visualizer/src/ARCHITECTURE.md`:
- Around line 7-12: The markdown diagram fences in ARCHITECTURE.md are unlabeled
and triggering markdownlint/pre-commit rewrites; update both affected fenced
code blocks to include a language tag such as text. Make the change consistently
for both the top architecture diagram block and the later block referenced in
the comment so the file no longer has bare triple-backtick fences.
In `@packages/command-visualizer/src/data/shapes.test.ts`:
- Around line 1-50: The file was reformatted by oxfmt during pre-commit, so
update the committed version of shapes.test.ts to match the formatter output.
Run the project formatter on the test module containing svgOf, the SHAPE_PATHS
suite, and the HATS_DIR declaration, then commit the resulting formatting-only
changes so CI stops failing.
In `@packages/command-visualizer/src/logic/build-render-object.ts`:
- Around line 1-118: The file formatting is out of sync with oxfmt, so update
build-render-object.ts to match the formatter output and ensure
buildRenderObject and flashRidesAfter keep the same behavior after reformatting.
Run the project formatter on this module, verify any import/order or spacing
changes it makes, and commit the formatted result so pre-commit no longer
rewrites the file.
---
Nitpick comments:
In `@packages/command-visualizer/src/render/css-cascade.ts`:
- Around line 131-134: The editor theme color values are duplicated between
cascadeThemeBridge() and themeVars(), so update the implementation to
single-source these values from one shared location instead of hardcoding them
twice. Move or export the editor color constants from a common module such as
data/colors.ts or data/decorations.ts, then have both css.ts’s themeVars() and
css-cascade.ts’s cascadeThemeBridge() reference the same symbols so the
dark/light editor styles stay in sync.
In `@packages/command-visualizer/src/render/jumbotron.ts`:
- Line 80: serializeJumbotron and its helpers recompute the timeline repeatedly;
compute timelineOf(state.frames) once, store the shared totalMs, and pass that
value into commandBar, metadataBlock, and dots instead of having each function
call timelineOf independently. Update the relevant helper signatures and their
call sites so they read the shared totalMs value for the style/metadata/dots
rendering paths.
In `@packages/command-visualizer/src/render/serialize-cascade.test.ts`:
- Line 27: Five regex literals in serialize-cascade.test.ts are missing the
unicode flag and are triggering require-unicode-regexp warnings. Update each
regex used in the serialize-cascade test helpers to include the u flag,
including the pattern in the html.matchAll call and the other referenced regex
literals in this test file. Keep the existing patterns and only add unicode
support so the lint warnings are cleared.
🪄 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: 2bda6c93-c4ae-4540-85d0-60f8f011ca7e
📒 Files selected for processing (46)
packages/command-visualizer/README.mdpackages/command-visualizer/UPSTREAM_REUSE.mdpackages/command-visualizer/package.jsonpackages/command-visualizer/src/ARCHITECTURE.mdpackages/command-visualizer/src/data/colors.tspackages/command-visualizer/src/data/decorations.tspackages/command-visualizer/src/data/shapes.test.tspackages/command-visualizer/src/data/shapes.tspackages/command-visualizer/src/index.tspackages/command-visualizer/src/logic/build-render-object.tspackages/command-visualizer/src/logic/chain.test.tspackages/command-visualizer/src/logic/chain.tspackages/command-visualizer/src/logic/derive-flashes.tspackages/command-visualizer/src/logic/derive-overlays.tspackages/command-visualizer/src/logic/fixture-extract.tspackages/command-visualizer/src/logic/fixture-root.tspackages/command-visualizer/src/logic/fixture-yaml.test.tspackages/command-visualizer/src/logic/fixture-yaml.tspackages/command-visualizer/src/logic/hat-allocator.test.tspackages/command-visualizer/src/logic/hat-allocator.tspackages/command-visualizer/src/logic/pipeline-types.tspackages/command-visualizer/src/logic/pipeline.tspackages/command-visualizer/src/logic/tokenize.tspackages/command-visualizer/src/model/columns.tspackages/command-visualizer/src/model/frame-state.tspackages/command-visualizer/src/model/overlays.tspackages/command-visualizer/src/model/timeline.tspackages/command-visualizer/src/render-command.tspackages/command-visualizer/src/render/css-cascade-flash.tspackages/command-visualizer/src/render/css-cascade.tspackages/command-visualizer/src/render/css.tspackages/command-visualizer/src/render/html.test.tspackages/command-visualizer/src/render/html.tspackages/command-visualizer/src/render/jumbotron-css-keyframes.tspackages/command-visualizer/src/render/jumbotron-css.tspackages/command-visualizer/src/render/jumbotron-shared.tspackages/command-visualizer/src/render/jumbotron.tspackages/command-visualizer/src/render/serialize-cascade.test.tspackages/command-visualizer/src/render/serialize-cascade.tspackages/command-visualizer/src/render/serialize.tspackages/command-visualizer/src/render/svg-wrap.tspackages/command-visualizer/src/render/symbols.tspackages/command-visualizer/tsconfig.jsonpackages/lib-engine/src/util/allocateHats/index.tstsconfig.base.jsontsconfig.json
💤 Files with no reviewable changes (3)
- packages/command-visualizer/src/render/html.test.ts
- packages/command-visualizer/src/logic/fixture-yaml.test.ts
- packages/command-visualizer/src/logic/fixture-root.ts
✅ Files skipped from review due to trivial changes (4)
- packages/command-visualizer/package.json
- tsconfig.json
- packages/command-visualizer/UPSTREAM_REUSE.md
- packages/command-visualizer/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/command-visualizer/tsconfig.json
- packages/command-visualizer/src/data/colors.ts
Comment on lines
+24
to
+26
| ``` | ||
| pre (bumper) → [ step.initial → step.during → step.final ]* → post (reset) | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the diagram fence.
This block is already tripping markdownlint MD040. Tag it as text so the docs stay lint-clean.
♻️ Proposed fix
-```
+```text
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/command-visualizer/README.md` around lines 24 - 26, The fenced
diagram in README is missing a language label and is triggering markdownlint
MD040. Update the fence around the diagram to be labeled as text so the
documentation remains lint-clean, and make the change in the README block
containing the pre/bump/post diagram.
Source: Linters/SAST tools
Comment on lines
+1
to
+118
| // Stage 3: generate render object. Extracted from pipeline.ts so each module | ||
| // stays under the 250-line ceiling. Assembles flashes, the DURING frame, and | ||
| // overlays from the tokenized before/after Frames into the CascadeState. | ||
| // Behavior identical to the inline stage that lived in fixtureToCascade. | ||
| // logic/ → logic/ import only. | ||
|
|
||
| import type { CascadeState, Frame } from "../model/frame-state"; | ||
| import type { GeneralizedRange } from "@cursorless/lib-common"; | ||
| import type { OverlayStyleName } from "../data/decorations"; | ||
| import { asArr, asObj, toGeneralizedRange } from "./fixture-extract"; | ||
| import { deriveFlashes } from "./derive-flashes"; | ||
| import { deriveOverlays } from "./derive-overlays"; | ||
| import type { | ||
| ParsedFixture, | ||
| PipelineOptions, | ||
| TokenizedStates, | ||
| } from "./pipeline-types"; | ||
|
|
||
| // Route a flash style to its native frame. | ||
| function flashRidesAfter(style: string): boolean { | ||
| return style === "justAdded"; | ||
| } | ||
|
|
||
| /** Assemble flashes, the DURING frame, and overlays into the CascadeState. */ | ||
| export function buildRenderObject( | ||
| parsed: ParsedFixture, | ||
| tokenized: TokenizedStates, | ||
| _opts: PipelineOptions = {}, | ||
| ): CascadeState { | ||
| const { theme, tabSize, meta, initial, final, ide } = parsed; | ||
| const { frames, beforeFrame, afterFrame } = tokenized; | ||
|
|
||
| const duringFlashes: { style: OverlayStyleName; range: GeneralizedRange }[] = | ||
| []; | ||
|
|
||
| // Step 6b (derived referenceFlashes) — see derive-flashes.ts. When a fixture | ||
| // records no ide.flashes but the doc changed, synthesize the pre-edit | ||
| // pendingDelete (rides DURING) + post-edit justAdded (rides AFTER) from the | ||
| // char-level prefix/suffix diff. Behavior identical to the inline version. | ||
| const recordedFlashes = asArr(ide?.flashes); | ||
| const initDoc = (initial?.documentContents as string) ?? ""; | ||
| const finDoc = | ||
| typeof final?.documentContents === "string" ? final.documentContents : null; | ||
| const derived = deriveFlashes({ | ||
| recordedFlashCount: recordedFlashes.length, | ||
| initDoc, | ||
| finDoc, | ||
| hasAfterFrame: afterFrame != null, | ||
| }); | ||
| duringFlashes.push(...derived.duringFlashes); | ||
| if (afterFrame) { | ||
| afterFrame.decorations.push(...derived.afterDecorations); | ||
| } | ||
|
|
||
| // Step 6: flashes. justAdded rides the AFTER frame (post-edit); every | ||
| // other flash is PRE-EDIT and rides the dedicated DURING frame (built | ||
| // below) — reference-class flashes sequence before deletion flashes there. | ||
| for (const f of asArr(ide?.flashes)) { | ||
| const fo = asObj(f); | ||
| if (!fo) { | ||
| continue; | ||
| } | ||
| const style = String(fo.style) as OverlayStyleName; | ||
| const range = toGeneralizedRange(asObj(fo.range) ?? {}); | ||
| if (!range) { | ||
| continue; | ||
| } | ||
| if (flashRidesAfter(style) && afterFrame) { | ||
| afterFrame.decorations.push({ style, range, role: "flash" }); | ||
| } else { | ||
| duringFlashes.push({ style, range }); | ||
| } | ||
| } | ||
|
|
||
| // Build the DURING frame (the execution beat — the instant the command | ||
| // pill goes active). ALWAYS present when the step has a final state, so | ||
| // every command occupies the same time scope. Content depends on flashes: | ||
| // - WITH pre-edit flashes: the initial doc, flashes firing (reference | ||
| // half then delete half) — the edit lands at the phase end. | ||
| // - WITHOUT flashes (pure selection commands like "take cap"): the edit | ||
| // is instantaneous in a real editor, so the during frame shows the | ||
| // FINAL state — the selection highlight lands in the SAME frame as the | ||
| // pill activation. | ||
| if (afterFrame) { | ||
| const instant = duringFlashes.length === 0; | ||
| const src = instant ? afterFrame : beforeFrame; | ||
| const duringFrame: Frame = { | ||
| role: "during", | ||
| lines: src.lines, | ||
| cursors: src.cursors, | ||
| selections: src.selections, | ||
| decorations: instant | ||
| ? [] | ||
| : duringFlashes.map(({ style, range }) => ({ | ||
| style, | ||
| range, | ||
| role: "flash" as const, | ||
| })), | ||
| clipboard: src.clipboard, | ||
| }; | ||
| frames.splice(1, 0, duringFrame); | ||
| } | ||
|
|
||
| // Step 7: highlights → BEFORE decorations, thatMark/sourceMark → AFTER | ||
| // decorations. See derive-overlays.ts; behavior identical to the inline | ||
| // version (order preserved: highlights, then that, then source). | ||
| const overlays = deriveOverlays({ | ||
| ide, | ||
| final, | ||
| hasAfterFrame: afterFrame != null, | ||
| }); | ||
| beforeFrame.decorations.push(...overlays.beforeDecorations); | ||
| if (afterFrame) { | ||
| afterFrame.decorations.push(...overlays.afterDecorations); | ||
| } | ||
|
|
||
| return { theme, tabSize, meta, frames }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix pre-commit formatting failure.
The pipeline reports oxfmt modified this file during pre-commit formatting, meaning the committed code doesn't match the expected format. Run the formatter and re-commit.
#!/bin/bash
# Run oxfmt on the file and re-commit
pnpm exec oxfmt packages/command-visualizer/src/logic/build-render-object.ts🧰 Tools
🪛 GitHub Actions: Pre-commit / Pre-commit
[error] 1-1: oxfmt modified this file during pre-commit formatting, indicating it did not match the expected format.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/command-visualizer/src/logic/build-render-object.ts` around lines 1
- 118, The file formatting is out of sync with oxfmt, so update
build-render-object.ts to match the formatter output and ensure
buildRenderObject and flashRidesAfter keep the same behavior after reformatting.
Run the project formatter on this module, verify any import/order or spacing
changes it makes, and commit the formatted result so pre-commit no longer
rewrites the file.
Source: Pipeline failures
…#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`).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New package:
@cursorless/command-visualizerAdds the command visualizer package that renders recorded command fixtures as standalone animated SVGs.
What this adds
@cursorless/command-visualizerpackage — renders recorded fixture YAMLs as animated SVGs.