Skip to content

fix(ot): fix some progress ops not being cleared - #607

Merged
twitchyliquid64 merged 1 commit into
mainfrom
tom/mip
Jul 1, 2026
Merged

fix(ot): fix some progress ops not being cleared#607
twitchyliquid64 merged 1 commit into
mainfrom
tom/mip

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Jul 1, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Found this sometimes happening with mip

Summary by CodeRabbit

  • Bug Fixes
    • Improved status updates during package checks and cache materialization, so completed work is now marked more reliably.
    • Fixed operation tracking so removed items are reflected promptly in progress views and renderers.
    • Added coverage for these tracking updates to help prevent regressions.

@coderabbitai

coderabbitai Bot commented Jul 1, 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: 9c1a5f70-d6d6-405b-b90d-7a6fbd921c08

📥 Commits

Reviewing files that changed from the base of the PR and between 73f88e3 and d5ea285.

📒 Files selected for processing (3)
  • crates/check/src/lib.rs
  • crates/ot/src/lib.rs
  • crates/rcache/src/remote.rs
📝 Walkthrough

Walkthrough

Adds a Drop implementation for TrackerInner that bumps the shared version when a node is dropped, with accompanying unit and async tests. Also adds explicit set_done() calls in check_package and RemoteCache::materialize after their respective operations complete.

Changes

Operation tracker completion fixes

Layer / File(s) Summary
Drop-driven version bump
crates/ot/src/lib.rs		 TrackerInner now implements Drop, bumping the shared version on node drop; new unit and Tokio async tests verify the root version increases and a watch receiver wakes on drop.
Explicit set_done calls
crates/check/src/lib.rs, crates/rcache/src/remote.rs		 check_package clones check_op before use and calls set_done() after collecting file-based checker results; RemoteCache::materialize calls materialize_op.set_done() after successful extraction.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant TrackerInner
  participant SharedState
  participant WatchReceiver
  Caller->>TrackerInner: drop node
  TrackerInner->>SharedState: bump()
  SharedState->>WatchReceiver: notify version change
  WatchReceiver->>Caller: changed().await resolves
Loading

Possibly related PRs

  • gominimal/minimal#515: Both PRs modify the operation-tracker core in crates/ot/src/lib.rs, with this PR extending that model via the new Drop implementation.

Suggested reviewers: 0chroma, evanspearman, norrietaylor

Poem

A tracker node hops off to rest,
Its drop now bumps the version's crest 🐇
No more silent, stuck-in-place —
set_done gives closure, wins the race!
Watchers wake with gentle cheer,
Little rabbit's work is clear. ✨

🚥 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 reflects the main change: fixing progress operations that were not being cleared.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@twitchyliquid64
twitchyliquid64 enabled auto-merge (squash) July 1, 2026 03:35
@twitchyliquid64
twitchyliquid64 merged commit 93e6929 into main Jul 1, 2026
37 checks passed
@twitchyliquid64
twitchyliquid64 deleted the tom/mip branch July 1, 2026 03:49
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

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