Repository navigation
Continue Sidekick from execution results with Follow - #1034
Conversation
PR #1034: Continue Sidekick from execution results with Follow22 files, +1196 / -198 Scope🔴 PR has 1394 lines changed. Split into focused PRs. 🟡 1394 lines changed. PRs under 400 receive more thorough review. 🟡 22 files changed. Are all changes related? StructuralOxlint structural signals:
Slop✅ Slop indicators look low. Static AnalysisOxlint: 180 diagnostics across 10 files (16 rules) no-shadow (95): packages/cli/src/repl/sidekick/admission.ts, packages/cli/src/repl/sidekick/dispatch.ts, packages/cli/src/repl/sidekick/requests.ts (+4) CorrectnessNo extraneous code patterns detected. |
taras
left a comment
There was a problem hiding this comment.
Full PR review of ec84362fd15df31fe2074f1ef8ee9b8474bc3a1e..4a75d1ff294543dd986960906c97e354620984dc: no actionable correctness, performance or minimalism findings.
The complete 22-file published diff preserves the accepted Follow decisions: a local default-on setting, Sidekick-submission-only continuation, eligible queued messages first, Sidekick Stop joining the Send while preserving independently admitted entries, and Entry Stop returning interrupted context after cleanup. I checked joined output against the retained terminal, Send-local interrupted text, fatal failure propagation, ordinary feedback with Follow off, and the finite CI repairs. The existing owner/cycle/dispatch structure remains appropriate; I found no concrete simplification that justifies changing accepted behavior. Shared TODO #901 remains deferred; this does not close the umbrella criterion.
Evidence: the supplied finite acceptance and affected repair evidence covers all three walkthroughs. I independently ran deno task test packages/cli/tests/repl-sidekick-journey.test.ts at this head: 14 tests / 54 steps passed. Exact-head CI run 41190579344 succeeded. Local passes are not evidence of a CI flake.
Performance measurement is relevant because mounted Follow input, evaluation-owned continuation and queued handoff affect application frame delivery. I ran only the two requested bounded workload families on the same macOS arm64 host, Deno 2.9.7 / V8 15.0.245.2-rusty, at 120×30 using the production mounted controls/input/fitter/renderer and deterministic provider transport, with no paid calls. Baseline checkout bb68fed64c99ac18153b8d2497c49c70f1daca23 independently verified tree-identical to the published base.
| Active workload | Inputs / completed application deliveries | Duration | Delivery FPS | Input-to-delivery p50 / p95 / worst |
|---|---|---|---|---|
| Baseline: queued composer editing | 20 / 20 | 2024.699 ms | 9.878 | 27.289 / 31.368 / 31.927 ms |
| Head: same editing | 20 / 20 | 2019.607 ms | 9.903 | 27.656 / 30.779 / 31.750 ms |
| Head: new Follow checkbox | 10 / 10 | 1012.629 ms | 9.875 | 14.060 / 15.029 / 15.029 ms |
Editing uses one held exchange, two fixed short pending messages, a 1,931-byte composer, an interior caret, six warmup x/Backspace inputs and 20 measured inputs at nominal 100 ms cadence. Queue labels/order, composer screen and the single held prompt remain unchanged. Toggle uses six warmup clicks and ten measured clicks at the same cadence; this new control has no baseline. Frame-interval p50 / p95 / worst: baseline editing 101.299 / 103.093 / 103.093 ms; head editing 100.775 / 103.174 / 103.174 ms; toggle 101.030 / 102.616 / 102.616 ms.
A separate head-only finite handoff phase releases one deterministic held response, completes two real short submitted entries through one queued message and automatic feedback, and preserves newer composer text. It delivered 21 application frames (20 nonempty) in 274.132 ms, or 76.605 deliveries/s during that burst; frame intervals were 9.220 / 26.637 / 42.624 ms. This phase has no terminal input, so no input-latency quantiles apply; it is not sustained interactive FPS or isolated join latency.
Raw timestamp samples and V8 CPU profiles were retained. Editing's leading nonidle stack in both trees is layout WASM → render/measure → batchWidths/cutAt → fitPlain/fitEditor → prepareFrame (leading leaf self time 149.929 ms base, 157.144 ms head across ~2.29 s profiles). The mixed Follow profile includes fitting, presentation and emulator cell copying; it does not isolate handoff CPU cost.
These are application write-completion deliveries, not native terminal flush or physical visible paint. Input latency uses the first post-input write as a delivery proxy; input-to-photon timing is unavailable. Startup, warmup, held-provider idle and final idle are excluded from active FPS. The small paced samples do not establish improvement, statistical significance, a general latency/memory guarantee or an FPS budget. V8 heap snapshots include harness/emulator/logs and cannot establish allocations or leaks. Initial inspector startup was sandbox-denied before work; the authorized run completed all workloads. All measurement children exited, and terminal readers/listeners returned to zero.
Follow starts checked in each REPL. Sidekick Send receives the exact submitted entry's completed, failed or interrupted outcome and actual output after cleanup, serves waiting UI/ACP messages before automatic work, and continues without another human Send.
Turning Follow off leaves the running entry and ordinary read feedback intact. Sidekick Stop ends its continuation while keeping the setting and leaving admitted entries independent; manual Draft Send creates no continuation. Interrupted text comes from the owning Send's final joined execution stream, while saved cancellation records stay unchanged.
Validation: the eight-file focused Follow matrix passes (70 tests, 297 steps), with precommit provenance and cosmetic post-test differences recorded. Refusal, output, UI-readiness and public-boundary corrections pass their explicit affected checks. The independent public Reviewer also ran the journey suite at this head: 14 tests and 54 steps pass. Required CI passes at combined head
5f2c9af8.The signed merge of accepted main
d10b2c2apreserves all 22 reviewed product-file blobs and the feature patch byte-for-byte; both accepted histories remain ancestors. The one-time public review found no actionable correctness, performance or minimalism issue. Two bounded interaction workloads retain timestamp samples and CPU profiles. At 100 ms paced input, equivalent queued-composer input-to-application-write p95 measured 31.37 ms on the source-equivalent baseline and 30.78 ms here; the new Follow checkbox measured 15.03 ms p95. These small application-delivery samples provide no general performance guarantee and do not measure native flush or visible paint.Refs #971. Shared TODO context remains deferred to #901; this delivery does not claim that umbrella criterion complete.