Skip to content

feat(ot)!: refactor ot to prepare for tracking operations per-session - #515

Merged
twitchyliquid64 merged 2 commits into
mainfrom
tom/session-destroy
Jun 21, 2026
Merged

feat(ot)!: refactor ot to prepare for tracking operations per-session#515
twitchyliquid64 merged 2 commits into
mainfrom
tom/session-destroy

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Jun 21, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Refactors the operations tracker to:

  • Have multiple roots (i.e. in the future there will be one per session)
  • Track all state internally rather than rely on storing some fields like progress in indicatif types
  • Separate the tree of operations from the render, and have an async-friendly notify
  • Move the render path for the legacy CLI into its own async task that uses the notify and the operation tree to print progress

A future PR can chain this up to minimald sessions and print progress down the ssh channel.

Summary by CodeRabbit

  • New Features

    • CLI now initializes a per-run operation tracker and renders operation progress to stderr for improved real-time visibility.
    • Operation rendering is now driven by a dedicated renderer shim, enabling progress output per session without interfering with stdout.

  • Chores

    • Refactored operation tracking to be render-agnostic, providing snapshot-based observation and async change signaling.
    • Made terminal progress rendering optional by gating it behind a feature flag.

@coderabbitai

coderabbitai Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b54fa4f1-8467-477c-835f-0b1d3e082b62

📥 Commits

Reviewing files that changed from the base of the PR and between 0c7eaf1 and cee9f10.

📒 Files selected for processing (1)
  • docs/specs/04-spec-ot-render-decoupling/04-spec-ot-render-decoupling.md
📝 Walkthrough

Walkthrough

Refactors crates/ot from an embedded indicatif-driven renderer into a render-agnostic operation-tree state tracker. Core types (OpId, Progress, OpSnapshot, RootShared) and APIs (snapshot, version, changed) replace the old progress-bar internals. A new indicatif_shim module handles rendering separately. indicatif is made an optional Cargo feature. The CLI wires in the new tracker at startup.

Changes

OpTracker render-agnostic refactor with indicatif shim

Layer / File(s) Summary
OpTracker state model and public APIs
crates/ot/src/lib.rs		 Introduces OpId, Progress, OpSnapshot, and RootShared (atomic counter + Notify). Redesigns TrackerInner to store operation/progress data and call shared.bump() on mutations. Adds new_root, snapshot/snapshot_into, version, and changed APIs. Updates tests for preorder ordering, ID uniqueness, version monotonicity, progress reset, independent roots, and changed() wakeup.
indicatif shim renderer
crates/ot/src/indicatif_shim.rs		 Adds a process-global MultiProgress, render_to_stderr entry point, and IndicatifShim async run loop that polls changed(), compares versions, and reconciles progress bars per OpSnapshot. Includes apply_progress, make_bar, op_style, op_tick, op_message helpers, StdoutWriter for stdout/MultiProgress coordination, and reconcile unit tests.
Cargo feature gating and CLI wiring
crates/ot/Cargo.toml, crates/minimal/src/main.rs		 Makes indicatif an optional dependency behind a new indicatif feature (default-enabled, also enables tokio/rt). Adds dev-dependency tokio with macros/rt/time. Wires OpTracker::new_root and render_to_stderr into run_cli, passing the tracker into ConfigBuilder via with_operation_tracker.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as run_cli
  participant OT as OpTracker
  participant Shim as IndicatifShim
  participant CB as ConfigBuilder

  CLI->>OT: new_root()
  CLI->>Shim: render_to_stderr(ot_root) — spawns async task
  CLI->>CB: with_operation_tracker(ot_root)

  rect rgba(100, 150, 255, 0.5)
    note over Shim,OT: Async render loop
    loop on each mutation
      OT-->>Shim: changed() wakes
      Shim->>OT: version(), snapshot()
      OT-->>Shim: Vec<OpSnapshot>
      Shim->>Shim: reconcile bars (add/update/rebuild/clear)
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 Hoppity-hop, the bars are now free,
No longer tangled in indicatif's tree!
A snapshot in time, a Notify ding,
The shim reconciles each glittering thing.
render_to_stderr — watch the spinners spin,
Render-agnostic, the tracker grows thin. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main refactoring objective: decoupling operation tracking from rendering to enable per-session tracking, which is the primary architectural change across multiple files in this changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

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

@gominimal-aw-bot

gominimal-aw-bot Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

This pull request has no accompanying spec. Comment /derive-spec to have one derived retrospectively from the code — it opens a separate spec/<slug> documentation PR with demoable units, acceptance criteria, and a gap analysis (implementation gaps, missing failure paths, weak acceptance criteria). Ignore this to defer; the weekly unspecced-PR scan will re-surface it. See ADR 0027.

@twitchyliquid64
twitchyliquid64 merged commit 94a5f8c into main Jun 21, 2026
46 checks passed
@twitchyliquid64
twitchyliquid64 deleted the tom/session-destroy branch June 21, 2026 21:27
This was referenced Jun 29, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@norrietaylor norrietaylor norrietaylor approved these changes
@0chroma 0chroma Awaiting requested review from 0chroma
@evanspearman evanspearman Awaiting requested review from evanspearman

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@twitchyliquid64 @norrietaylor