Perf by vibe by tiye · Pull Request #40 · Cirru/parser.ts

tiye · 2026-03-10T17:07:16Z

No description provided.

…imize lexer perf (~34% faster)

… skip dollar/comma passes

…array allocation

…d lexAndBuild

…ents

Copilot

Pull request overview

This PR focuses on improving parser performance by eliminating intermediate allocations during lex/parse and by merging the $ and , tree-transform passes, while also adding a simple benchmark harness and updating project tooling/CI for Yarn 4.

Changes:

Replace the previous token-buffer-based pipeline with a single-pass lexAndBuild() parser in src/index.ts.
Optimize tree transforms in src/tree.ts (no-slice helpers + new combined resolveDollarComma) and add equivalence tests.
Add benchmarking support (src/bench.ts, tsconfig.bench.json, yarn bench) and update Yarn/CI/docs.

Reviewed changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`src/index.ts`	New single-pass lexer+builder and conditional `$`/`,` transform invocation.
`src/tree.ts`	No-slice comma/dollar helpers + new combined `resolveDollarComma` pass.
`src/parser.test.ts`	Adds equivalence tests to validate combined `$`/`,` behavior.
`src/bench.ts`	Adds a benchmark runner for local perf testing.
`tsconfig.bench.json`	Adds a bench-specific TS config (Node types, ES2022 target).
`package.json`	Adds `bench` script, bumps version, updates dev deps, pins Yarn via `packageManager`.
`Agents.md`	Adds contributor/agent perf/testing/tooling notes.
`.yarnrc.yml`	Configures Yarn to use `node-modules` linker.
`.gitignore`	Ignores bench artifacts and Yarn state files.
`.github/workflows/upload.yaml`	Updates Node setup + Yarn caching/immutable installs + uses `yarn compile`.
`.github/workflows/npm-publish.yaml`	Updates Node setup + Yarn caching/immutable installs + uses `yarn compile`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T17:16:00Z

Agents.md

+```bash
+yarn test          # run all 22 Jest tests
+yarn bench         # benchmark on bundled test fixtures (×20, ~16 KB)
+BENCH_FILE=/absolute/path/to/large.cirru yarn bench   # benchmark on a real file


This doc line hard-codes "run all 22 Jest tests", but src/parser.test.ts currently defines more than 22 tests (and later in this document it also mentions 27). Consider avoiding a specific count here (or update it) so it doesn't go stale.

Copilot · 2026-03-10T17:16:00Z

Agents.md

+| File                    | Purpose                                                     |
+| ----------------------- | ----------------------------------------------------------- |
+| `src/index.ts`          | Parser entry point; `lexAndBuild`, `parse`, `parseOneLiner` |
+| `src/tree.ts`           | Tree helpers: `resolveDollar`, `resolveComma`, utilities    |
+| `src/types.ts`          | `ELexState`, `ELexControl`, `ICirruNode`                    |
+| `src/parser.test.ts`    | All 22 tests                                                |
+| `src/bench.ts`          | Benchmark script                                            |
+| `tsconfig.bench.json`   | Separate tsconfig for bench (es2022, node types)            |
+| `tsconfig-compile.json` | Library build config                                        |
+| `test/cirru/*.cirru`    | Test input fixtures                                         |
+| `test/ast/*.json`       | Expected AST outputs                                        |
+| `lib/`                  | Compiled library output (committed)                         |


The File Map claims lib/ is "Compiled library output (committed)", but .gitignore ignores lib/* and CI builds lib/ via yarn compile. Please clarify whether lib/ is expected to be committed or generated, and update this row accordingly to avoid confusing contributors.

Suggested change

| File | Purpose |

| ----------------------- | ----------------------------------------------------------- |

| `src/index.ts` | Parser entry point; `lexAndBuild`, `parse`, `parseOneLiner` |

| `src/tree.ts` | Tree helpers: `resolveDollar`, `resolveComma`, utilities |

| `src/types.ts` | `ELexState`, `ELexControl`, `ICirruNode` |

| `src/parser.test.ts` | All 22 tests |

| `src/bench.ts` | Benchmark script |

| `tsconfig.bench.json` | Separate tsconfig for bench (es2022, node types) |

| `tsconfig-compile.json` | Library build config |

| `test/cirru/*.cirru` | Test input fixtures |

| `test/ast/*.json` | Expected AST outputs |

| `lib/` | Compiled library output (committed) |

| File | Purpose |

| ----------------------- | ----------------------------------------------------------------------- |

| `src/index.ts` | Parser entry point; `lexAndBuild`, `parse`, `parseOneLiner` |

| `src/tree.ts` | Tree helpers: `resolveDollar`, `resolveComma`, utilities |

| `src/types.ts` | `ELexState`, `ELexControl`, `ICirruNode` |

| `src/parser.test.ts` | All 22 tests |

| `src/bench.ts` | Benchmark script |

| `tsconfig.bench.json` | Separate tsconfig for bench (es2022, node types) |

| `tsconfig-compile.json` | Library build config |

| `test/cirru/*.cirru` | Test input fixtures |

| `test/ast/*.json` | Expected AST outputs |

| `lib/` | Compiled library output (generated; not committed—run `yarn compile`) |

Copilot · 2026-03-10T17:16:00Z

Agents.md

+
+- Framework: Jest + `ts-jest`
+- Test file: `src/parser.test.ts`
+- Fixtures: `test/cirru/*.cirru` (source) + `test/ast/*.json` (expected output)
+- Every optimization must keep all **27 tests passing**.


The testing section states "keep all 27 tests passing", but src/parser.test.ts currently has 26 test(...) cases. To avoid future drift, consider removing the exact number here (or ensure it matches the actual Jest test count).

Copilot · 2026-03-10T17:16:01Z

Agents.md

+| `src/index.ts`          | Parser entry point; `lexAndBuild`, `parse`, `parseOneLiner` |
+| `src/tree.ts`           | Tree helpers: `resolveDollar`, `resolveComma`, utilities    |
+| `src/types.ts`          | `ELexState`, `ELexControl`, `ICirruNode`                    |
+| `src/parser.test.ts`    | All 22 tests                                                |


The File Map entry for src/parser.test.ts says "All 22 tests", but the test file currently contains more than 22 test cases. Consider removing the specific number (or updating it) so the file map stays accurate over time.

Suggested change

| `src/parser.test.ts` | All 22 tests |

| `src/parser.test.ts` | Jest tests for the parser |

Copilot · 2026-03-10T17:16:01Z

src/index.ts

+          case CHAR_LPAREN:
+            if (isOdd(indentCount)) throw new Error(`Invalid indentation size ${indentCount}`);
+            flushIndent(indentCount >> 1);
+            emitOpen();
+            state = ELexState.space;
+            indentCount = 0;
+            break;
+          default:
+            if (isOdd(indentCount)) throw new Error(`Invalid indentation size ${indentCount}`);
+            flushIndent(indentCount >> 1);
+            state = ELexState.token;
+            tokenStart = pointer - 1;
+            indentCount = 0;
+            break;


In ELexState.indent, a leading ) is currently treated as the start of a token (falls through to default) instead of emitting a close paren. This makes ) behave inconsistently depending on lexer state and can break inputs where a line begins with ) (it would produce a literal ")" token instead of closing a list). Add a CHAR_RPAREN case here that mirrors the CHAR_LPAREN handling (validate indent, flushIndent(...), then emitClose() and switch to space).

Copilot · 2026-03-10T17:16:01Z

src/index.ts

+  const emitClose = () => {
+    const completed = current!;
+    if (stack.length === 0) {
+      result.push(completed);
+      current = null;
    } else {
-      throw new Error(`Unexpected chunk ${JSON.stringify(chunk)}`);
+      current = stack.pop()!;
+      current.push(completed);
    }
-  }
-};
+  };


emitClose() uses a non-null assertion (current!) and will throw a runtime TypeError if a close paren or indentation-driven close is encountered when current is null (e.g., malformed input with an extra )). The previous implementation threw a controlled parse error for unexpected closes; consider guarding emitClose() and throwing an explicit Error (e.g. unexpected ")").

Copilot · 2026-03-10T17:16:01Z

src/index.ts

+            state = ELexState.escape;
+            break;
+          case CHAR_NEWLINE:
+            throw new Error("Expected newline in string");


The error message thrown on newline inside a string literal is misleading: throw new Error("Expected newline in string") is triggered because a newline was encountered before the closing quote (i.e., an unterminated/invalid string). Consider updating the message to something like "Unexpected newline in string" or "Unterminated string literal" to match the actual error condition.

Suggested change

throw new Error("Expected newline in string");

throw new Error("Unexpected newline in string");

tiye added 9 commits March 11, 2026 00:04

switch to yarn berry (node-modules linker), upgrade vite to beta, opt…

f9781a8

…imize lexer perf (~34% faster)

replace esbuild with tsc+node for bench script; add tsconfig.bench.json

ee8980d

update CI workflows for yarn berry: corepack, --immutable, builtin cache

a3c7ef9

optimize lexer: single-pass lex+indent, slice-based token extraction,…

c441701

… skip dollar/comma passes

optimize dollar/comma helpers: pass bounds instead of slice to avoid …

f7e4fb9

…array allocation

eliminate tokens[] array: merge lex+build into single-pass stack-base…

80ced1b

…d lexAndBuild

add Agents.md: dev/test/optimization notes for contributors and AI ag…

9646694

…ents

optimize lexer dispatch with charCodeAt and refresh profiling notes

89a07a2

combine dollar/comma transforms with equivalence coverage

39b5614

tiye requested review from a team and Copilot March 10, 2026 17:07

Copilot started reviewing on behalf of tiye March 10, 2026 17:07 View session

tiye force-pushed the perf branch 2 times, most recently from a838967 to e3919cc Compare March 10, 2026 17:11

fix perf; bump 0.0.9

2bd75d9

tiye force-pushed the perf branch from e3919cc to 2bd75d9 Compare March 10, 2026 17:13

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Perf by vibe#40

Perf by vibe#40
tiye wants to merge 10 commits intomainfrom
perf

tiye commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	\| `src/parser.test.ts` \| All 22 tests \|
	\| `src/parser.test.ts` \| Jest tests for the parser \|

	throw new Error("Expected newline in string");
	throw new Error("Unexpected newline in string");

Conversation

tiye commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants