Conversation
The cold-park verdict flip damper only engaged on a burst: 3 flips inside a 1s
window, a threshold derived from React's 50-commit nested-update bail. The
notice limit — 12 flips in 60s — was explicitly breadcrumb-only, on the
reasoning that churn that slow was never near React's bail and damping it would
spend a mounted pane's memory for no crash-safety gain.
The field disagrees about the cost, though not for the reason first recorded.
WHAT THE BUNDLE SHOWS
Bundle Nz4kzIG_NwLd8KObgjJDKA (v1.4.201, win32) carries 52 park-verdict churn
crumbs. They are two unrelated episodes across four launches, not one run:
L3 09-14 00:53 35 crumbs, 1 tab, 46.9 min, 34 window + 1 burst
L4 09-14 17:07 17 crumbs, 4 tabs, 8.9 min, 17 window + 0 burst
L3's 46.9 min counts a lone trailing burst; the window-triggering run itself is
35.2 min, with an 11.7 min quiet gap before that final burst.
Per-flip cadence is elapsedMs / (NOTICE_LIMIT - 1), not / NOTICE_LIMIT: the
window opens ON a flip, so the 12th sits 11 intervals later. L3 runs 3.19s /
4.63s / 5.44s per flip (min/median/max); L4 runs 3.19 / 3.20 / 5.01.
Across 321 field bundles, 24 carry this churn and all 24 burst at least once —
though those 24 are roughly 17 devices, and 23 of them are burst-only, so the
load-bearing fact is narrower than it sounds: this install is the ONLY one that
ever tripped the window rule, and that is what ran undamped.
WHAT IT COSTS, STATED ACCURATELY
Each flip remounts the pane. A remount does NOT reconnect a terminal: parking
deliberately keeps the PTY alive (terminal-parked-tab-watchers runs a pane-less
byte watcher), SSH restores from main's snapshot, and a remote runtime
re-subscribes a stream on a per-environment multiplexer that outlives the pane.
The cost is a remount each time, indefinitely.
The user who filed wrote "The connection closes, reconnects and again and again
and again all the time. Is a cycle." An earlier draft of this change took that
as the mechanism. It is not: the bundle carries no remote-terminal stream-stall
recovery crumbs, no daemon session churn inside either churn window, and one
pane-recovery remount two hours earlier. And the report was filed in L4, 12.3
minutes AFTER that launch's churn had already stopped. The churn is real and
worth damping on its own terms; the causal link to that sentence is not
established, and this change should not be read as proving it.
THE FIX
The notice limit now engages the same unpark pin the burst does, backing off ×2
per consecutive notice window to an 8-minute ceiling, clearing once a full
window closes below the limit.
Measured closed-loop — driving the pin back into the verdict the way
useTerminalParkVerdictPin does — over 47 simulated minutes, counting pane
remounts, which is the user-visible quantity:
per flip transitions before -> after ceiling reached
3193 ms 884 -> 96 (9.2x fewer) 8x
4627 ms 610 -> 96 (6.4x fewer) 8x
4800 ms 588 -> 96 (6.1x fewer) 8x
5000 ms 564 -> 126 (4.5x fewer) 8x
5442 ms 519 -> 156 (3.3x fewer) 8x
These count verdict TRANSITIONS; a mount/unmount cycle is two, so actual pane
mounts are half each figure. The ratios are unaffected. The harness applies the
pin in the same commit cascade as the flip that caused it, which is what the
hook does and what three rounds of review showed an open-loop harness cannot
see.
TRADE-OFFS AND LIMITS, NONE OF THEM HIDDEN
- This does not fix the driver. Whatever keeps re-proposing the park is still
unidentified; the pin masks the rendered verdict and the driver resumes when
each pin lapses. This caps the remount rate of an oscillation; it does not end
one.
- A pinned pane stays mounted and holds its memory, re-armed while churn
persists. One oscillating per-worktree flag flips every tab in that worktree
in the same pass, so all of them can pin at once. The merged verdict includes
the retention-budget force-park, so a churn-pinned pane can stay mounted
through memory reclamation that was trying to free it — and the reclaimer
records success either way, because its only anomaly breadcrumb fires on a
different condition.
The honest number is a duty cycle, not a ceiling. Measured per cadence:
89.7% of wall time pinned at 3193ms/flip, 84.3% at 4627, 83.9% at 4800,
77.8% at 5000, 69.1% at 5442, with a peak contiguous pin of ~480s. Before this
change none of these cadences pinned at all, so blocked reclamation goes from
roughly 0% to 69-90%. Repo-stated cost is ~2.5MB per mounted pane at the 5k
scrollback default and ~19MB at 50k. This is the strongest form of the
original "memory is not free" objection and the change does not answer it.
- Only the notice path backs off; a burst still takes a flat one-window pin.
The corpus supports leaving it: repeat bursts arrive a median 673s apart
(n=29, grouped by bundle/launch/tab; 356s if physical pairs are deduped), and
1 of those 29 inter-arrivals falls in the band that would mean re-firing the
instant a pin lapsed. That margin is thin — the next gap above the band is
75.3s, 0.3s outside it.
- One hard edge, geometric: reaching 12 flips takes 11 intervals, so churn
slower than 60s/11 = 5454.5ms per flip never reaches the limit at all. The
field's slowest observed episode runs 5441.8ms, 12.7ms inside it. Past roughly
4.7s per flip each window only barely makes the limit, so damping engages
later and the win shrinks from 9.2x to 3.3x — but the back-off reaches the
8-minute ceiling at every cadence measured.
- Crumb volume drops by roughly 6x, a loss of
telemetry sensitivity for the very corpus scan that found this. The surviving
'window' crumbs carry sustainedPinCount, which discriminates sustained churn
better than raw volume did; 'burst' crumbs do not carry it. The detection
method changes either way.
REVIEW: TWO ROUNDS, AND BOTH FOUND THE FIX ITSELF
Round 1 found the back-off never left 1x in production. While a tab is pinned
the hook subtracts it from the rendered verdict, so no flip is recorded and
windowStartMs never advances; by the time a pin lapsed, windowStartMs was always
older than a full window, so the quiet branch wiped sustainedPinCount on the
first pass after EVERY pin. Closed-loop, that shipped 3.6x the remounts
intended. The 24 tests then in the file passed either way, because the harness
was open-loop. A second lens found five wrong claims in the commit message,
including a per-flip divisor of 12 where the code uses 11 — which made the
stated safety margin 37x too generous — and a reconnect mechanism the source
contradicts.
Round 2 attacked round 1's fix and found two more:
- The back-off RATCHETED. The flip path cleared it on `flips < noticeLimit`,
and the notice path leaves `flips` at exactly noticeLimit, so a window
reopening after hours of silence did not clear it. Four isolated episodes
six hours apart reached the 8-minute ceiling — the precise behaviour the
code comment claimed was impossible.
- And it was INERT above ~4.7s per flip, because a lapsing pin knocks the next
window out of alignment so it closes at 11 of 12, which that same
`flips < noticeLimit` test read as the churn easing. At the field's slowest
observed cadence round 1's fix changed nothing at all: 519 -> 346 both with
and without it.
Both come from one wrong question. The reset now asks whether the tab has been
genuinely quiet — a full window with no flip and no live pin, tracked on an
explicit lastFlipMs — or whether the closing window held fewer than half the
notice limit. A window closing at 11 of 12 is neither.
Round 3 found the defect inside round 2's fix, and it was one line. Engaging the
pin IS a rendered verdict transition: the hook drops the tab from the parked
set, the effect re-runs on the changed dep, and that flip lands while the pin is
live — overwriting the future deadline with the current time. The back-off then
cleared one window after the pin STARTED rather than one window after it ended.
Round 3 also showed the residual slow-cadence weakness was this bug and not the
geometry the round-2 message blamed, and that reverting to round 2's threshold
still passed all 31 tests. `lastFlipMs` no longer moves backwards off a live
deadline, and a closed-loop test that records a flip inside a live pin now kills
both that clobber and round 2's threshold.
Round 4 found no defect inside round 3's fix and judged it mergeable. It did
find that the eased-flip clause was itself untested — four mutations of it,
including reverting to round 2's threshold, passed every test — and that the
test whose comment claimed to cover it was satisfied by the other disjunct. A
sparse-churn test (three flips per window, indefinitely: eased but never silent)
now separates the two and kills all four.
Also from the review rounds: two separate dead clauses in this function, one
added by each of the previous two rounds, are gone; the burst-interval median in
the shipped comment was one row off (673s, not 732s) and n is 29, not 30. That
median is grouping-fragile — 702.7s by (bundle,tab), 356.1s deduped — but the
facts it supports are not: 1 value in the 59-75s band under every grouping, and
the next gap above it is 75.3s.
Known and not fixed: a tab churning continuously at 6-10 flips per window sits
in the gap between both reset conditions and holds its back-off indefinitely, so
the next notice after it speeds up pins for the ceiling rather than one window.
Stated rather than fixed: because quiet is measured from the pin deadline, a tab
at the 8-minute ceiling needs roughly nine minutes of real silence before the
back-off clears, not one window. The record's doc says so.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughTerminal park flip telemetry now tracks sustained churn and applies exponentially increasing pin durations up to a configured maximum. Activity uses flip and pin deadlines, and sustained state resets after quiet or reduced-churn windows. Tests cover cadence thresholds, closed-loop churn, back-off, maximum duration, isolated episodes, and reset behavior. A test helper now supplies the required Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The sustained pinning change has no identified unresolved issue in the supplied review context and is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description gives detailed context, rationale, trade-offs, evidence, and test coverage. However, it omits required template sections, including Linked Issue, Visual Proof or an explicit N/A explanation, Testing details and checklist, Agent skill upstream boundary, Notes, and the final Checklist. Resolution Add the missing template sections. Include a linked issue after “Fixes #”, state “N/A” with a reason if there is no visual change, document testing steps and tested platforms, complete the Agent skill upstream boundary section, address the Notes items, and complete the checklist. Add AI Disclosure and Review content when applicable.
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
ℹ️ No critical issues — two documentation nits inline. I independently re-ran the changed suites (38 tests, green), typecheck, and lint, and reproduced the closed-loop damping claims below the window cliff.
Reviewed changes
- Sustained-churn damping — the notice limit (12 flips/60s) now engages the unpark pin instead of only breadcrumbing, with exponential back-off (
60s → 8min) tracked bysustainedPinCount. - Quiet-activity tracking — new required
lastFlipMsrecords a flip or a suppressing pin's deadline, so the back-off resets only after a genuinely quiet window and cannot ratchet across isolated episodes. - Pin-clobber fix —
lastFlipMs = max(nowMs, pinnedUntilMs)keeps a flip that lands inside a live pin (pin engagement is itself a transition) from overwriting the deadline and clearing the back-off a window early. - Test coverage — added an open-loop regression guard plus a closed-loop harness that feeds the pin back into the rendered verdict, pinning the clobber, the alignment cliff, the 8× ceiling, and both reset conditions.
I verified the state machine independently: a 47-minute closed-loop run at the 5442 ms/flip slow end reaches the ceiling (sustainedPinCount 7, max pin 480000), and a 2000-5454 ms sweep in 20 ms steps found no cadence where the back-off stays at the first 60s pin. The documented cliff sits between 5442 and 5460 ms.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
| * | ||
| * Only the notice path backs off. A burst still takes a flat one-window pin, | ||
| * which the corpus supports: repeat bursts arrive a median 673s apart (n=29, | ||
| * grouped by bundle/launch/tab; 356s if physical pairs are deduped), and just |
There was a problem hiding this comment.
The corpus stats here (median 673s, n=29, 1 of those 29) don't match the PR description (median 732 s apart, 1 of 30 inter-arrivals). Both describe the same burst corpus, so one is stale — worth reconciling so the sample size and median a future reader trusts are the real ones.
| // looks like no flips at all, so a flip-gated reset would ratchet — a tab | ||
| // that churned once at launch would still carry a ceiling pin hours on. | ||
| // | ||
| // Why no isParkVerdictPinLive check: a live pin puts quietSinceMs in the |
There was a problem hiding this comment.
quietSinceMs is not a field on ParkVerdictFlipRecord; this refers to lastFlipMs. Looks like a stale name from an earlier revision.
| // Why no isParkVerdictPinLive check: a live pin puts quietSinceMs in the | |
| // Why no isParkVerdictPinLive check: a live pin puts lastFlipMs in the |

What this fixes
The cold-park verdict flip damper only engaged on a burst — 3 flips inside 1s, a threshold derived from React's 50-commit nested-update bail. The notice limit (12 flips in 60s) was breadcrumb-only, on the reasoning that churn that slow was never near React's bail and damping it would spend a mounted pane's memory for no crash-safety gain.
That reasoning only weighed crash safety. It never weighed the remounts.
ELI5
Orca decides, per terminal tab, whether to "park" it — unmount the pane to save memory. For one user that decision kept flip-flopping: park, unpark, park, unpark, every few seconds, for 47 minutes straight. Every flip tears the pane down and rebuilds it.
There was already a brake, but it only caught fast flip-flopping — three flips in one second. This user's was slow: one flip every three to five seconds. Fast enough to be miserable, too slow to trip the brake. This PR makes the slow case trip a brake too, and the brake gets longer each time the tab refuses to settle (1 min, 2, 4, up to 8), resetting once the tab calms down.
Fix evidence
Field bundle
Nz4kzIG_NwLd8KObgjJDKA(v1.4.201, win32) carries 52 churn crumbs across two episodes in two launches:Per-flip cadence is
elapsedMs / (NOTICE_LIMIT - 1)— the window opens on a flip, so the 12th sits 11 intervals later. L3: 3.19 / 4.63 / 5.44 s (min/median/max).Across 321 field bundles, 24 carry this churn and all 24 burst at least once, so the burst damper reached every one. This install is the only one that also sustains window-limit churn — that is the part that ran undamped.
Measured closed-loop (driving the pin back into the verdict the way
useTerminalParkVerdictPindoes), 47 simulated minutes, counting pane remounts — the user-visible quantity:These count verdict transitions; a mount/unmount cycle is two, so pane mounts are half each figure.
Against the pre-PR source, 9 of the 33 tests fail.
Trade-offs — none hidden
sustainedPinCount, which discriminates sustained churn better than raw volume, but the detection method changes.What this PR does not claim
The user wrote "The connection closes, reconnects and again and again and again." An earlier draft took that as the mechanism. It is not. A remount does not reconnect a terminal — parking deliberately keeps the PTY alive, SSH restores from main's snapshot, and a remote runtime re-subscribes a stream on a per-environment multiplexer that outlives the pane. The bundle carries no remote-terminal stream-stall crumbs and no daemon session churn in either window, and the report was filed 12.3 minutes after that launch's churn had already stopped. The churn is real and worth damping on its own terms; the causal link to that sentence is not established.
Adversarial review: 4 rounds, clean on round 4
Rounds 1–3 each found the round's worst defect inside the previous round's fix. Every lens ran in its own worktree pinned to a commit SHA.
windowStartMsnever advances — so the quiet branch wiped the counter on the first pass after every pin. Shipped 3.6× the intended remounts. All 24 tests passed either way, because the harness was open-loop. A second lens found five wrong claims in the message, including a per-flip divisor of 12 where the code uses 11 — making the stated safety margin 37× too generous — and a "reconnect" mechanism the source contradicts.flips < noticeLimit, and the notice path leavesflipsat exactlynoticeLimit, so four isolated episodes six hours apart reached the 8-minute ceiling. It was also inert above ~4.7 s/flip — at the field's slowest cadence round 1's fix changed nothing (519 → 346 with and without it).The test gap mattered more than any single bug.
easedFlipCount → noticeLimit— behaviourally round 2's defective revision — passed all 31 tests. The closed-loop test that records a flip inside a live pin is the sole killer of both that and round 3's clobber, and a sparse-churn test now covers the remaining clause. Four mutants that survived round 4 all die.