Repository navigation
Conversation
Add projection micro-benchmarks and regression coverage for the streaming markdown projection layer now that @opencode/patchmark (anomalyco#57016) owns incremental parsing. The benchmarks stream deterministic fixtures through project() and record wall time, per-update latency, and patchmark's charsScanned. The README compares the patchmark path with the pre-anomalyco#57016 marked baseline on the same fixtures. The tests cover projection behavior that patchmark's own suite does not assert directly: frozen-block identity, partition integrity, incremental vs full projection, blank-line splits across deltas, and code-mode switching.
There was a problem hiding this comment.
No blocking problems. This adds tests and benchmarks only. I ran the new tests (59 pass), bun test src in packages/session-ui (331 pass), typecheck:tsc (clean) and both bench scripts. The scan counts and partition results match the README; the times were lower on my machine, as expected.
Public API: unchanged (tests and benchmark scripts only).
Worth fixing:
- Several new tests are named after #55857's live-tail approach, where there was a cap and an "oversized tail" (see inline). The current
project()just forwards toPatchmarkStream.update, so nothing has a cap. Please rename them to what they actually check, or drop them. - The
scale.bench.tsheader says per-delta work stays bounded, but its own output shows scanned characters growing with the square of the text length (see inline). - The README's "Marked baseline" column came from a harness with
markedlexer counters that isn't in this PR, so nobody can regenerate it. Please either explain briefly how it was produced or keep only the current numbers.
| }) | ||
| }) | ||
|
|
||
| test("boundedly re-lexes the live tail once it exceeds the cap", () => { |
There was a problem hiding this comment.
There's no cap in the current project(): it forwards every update to PatchmarkStream.update. This test and the three "oversized tail" tests below carry over the names (and the 400–600-step loops) from #55857's approach, which no longer exists. As written, this one only checks that the blocks join back into the text and that at least one block is settled; the partition test further down already covers that. Could you rename these to the patchmark behavior they actually protect (for example, a blank line split across two deltas still splits the block), shrink the loops, and drop any that add nothing?
| // Scaling sweep for `project()` in `src/components/markdown-stream.ts`. | ||
| // | ||
| // Streams one boundary-less paragraph of N words (one word per delta) and | ||
| // records total projection time plus the characters scanned by patchmark. The |
There was a problem hiding this comment.
This says per-delta work stays bounded as the text grows. This script's own output contradicts that: charsScanned goes from 197,240 to 12,630,526 between 250 and 2,000 words, so it grows with the square of the length, as the README says. Please update the comment so it matches what the sweep measures.
Rename the live-tail tests to describe patchmark behavior instead of the removed cap, shrink their loops, and state in the scale benchmark header that a boundary-less line rescans the whole active line each delta. Record how the marked baseline was produced so the README numbers can be traced, and note that the baseline harness is not part of this PR.
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
Addressed the review in
|
There was a problem hiding this comment.
Both earlier points are fixed, plus the README baseline note. The leftover test names are now named after what they check, with shorter loops. The scale.bench.ts header matches what the sweep measures. The README now explains how the "Marked baseline" column was produced. Re-ran markdown-stream.test.ts (59 pass), bun test src (331 pass) and typecheck:tsc (clean). Nothing new to raise.
Public API: unchanged (tests and benchmark scripts only).
There was a problem hiding this comment.
The new commit only merges dev and doesn't change this PR's files. Both earlier points were fixed in the previous push. Re-ran markdown-stream.test.ts (59 pass) and typecheck:tsc (clean) on the merged head. Nothing new to raise.
Public API: unchanged (tests and benchmark scripts only).
Issue for this PR
Refs #56202 (diagnosis), #55857 (prior implementation), #57016 (replacement).
Type of change
What does this PR do?
Adds benchmarks and regression tests for the streaming Markdown projection in
packages/session-ui. No production code changes.#57016replaced themarked+remendprojection with@opencode/patchmark. The benchmarks stream deterministic fixtures throughproject(previous, text, true)and record wall time, per-update latency, and patchmark'scharsScanned. The tests cover projection behavior that patchmark's own suite does not assert: frozen-block identity, partition integrity, incremental-vs-full equivalence, blank-line splits across deltas, code-mode switching, and non-append fallbacks.This is the surviving half of
#55857. That PR targeted the removedmarkedpath, so its implementation and the tests tied to it are gone.Representative run (Ryzen Threadripper 1920X, Bun 1.4.2, 3 measured passes, medians). Baseline is
f2735ef79b^, the commit before#57016, run through the same fixtures and timing loop:many-paragraph-deltasone-long-paragraph(12.6 KB)completed-head-live-tailmixed-long-responsegrowing-listgrowing-tableopen-code-fenceThe open-code-fence row is a measured regression of ~3.4 ms across 401 updates, reported rather than hidden. How the baseline was produced, the scaling sweep, and limitations are in
packages/session-ui/performance/markdown-stream/README.md.Provenance
#48432opened.thdxr: "If you think this is still relevant, please port it over and open it for V2." No review.#55857opened. Benchmarks: 15.9× speedup (145.3 ms → 9.1 ms) and 114.3× speedup (10,565.2 ms → 92.5 ms). Theopencode-agentreview acknowledged "The speed-up is real" and asked for the split-delta fix.37402d29fixed the split-delta case;#56202filed. The bot reproduced it the same day and noted "Your open PR fix(session-ui): optimize incremental markdown projection #55857 targets this path."#55857.#57016did not exist until less than 72 hours after my final commit to#55857; it was then created, merged, and replaced themarked+remendpath.#55857closed as superseded; this PR carries the benchmarks and tests that still apply.#57016's implementation is its own — no code was copied. The credit being asked for is the diagnosis (#56202) and the measurements, which predate#57016.How did you verify your code works?
bun test srcinpackages/session-ui— 331 pass / 0 fail.bun run typecheck:tscinpackages/session-ui— clean.bunx oxlinton the changed files — 0 warnings / 0 errors.f2735ef79b^for the baseline column.Visual
No user-facing change.
Screenshots / recordings
None; no UI change.
Checklist