Skip to content

feat: pre-warm drill targets — no empty-view flash on drill push/pop - #160

Merged
hellices merged 4 commits into
mainfrom
feat/drill-prewarm
Aug 2, 2026
Merged

feat: pre-warm drill targets — no empty-view flash on drill push/pop#160
hellices merged 4 commits into
mainfrom
feat/drill-prewarm

Conversation

@hellices

@hellices hellices commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Closes #157

Problem

Drilling deploy → rs → pods (and Esc back) read as two visible steps per transition: jump to an empty child view, then rows pop in a beat later. Root cause: _drill_into switched the pane first; WatchManager.start then cleared the bucket and began the LIST, so the first render of the child view was always against an empty store, and content arrived one network RTT later. _pop_drill had the same shape (the parent watch was stopped on the way down → re-clear + re-LIST on the way back).

Fix — pre-warm the target view before switching

_drill_into / _pop_drill (and everything routed through them: Enter, Esc, agent_drill_down, helm h history) now:

  1. Start the target watch while the current view is still up — a kind no pane displays renders nowhere, so the LIST happens invisibly.
  2. Wait, bounded by DRILL_PREWARM_TIMEOUT (1s), until the rows the transition will show existowned_by(parent_uid) rows for a push, any parent row for a pop. All three drill chains qualify (replicasets, pods, and helm helmrevisions all carry owner_uids).
  3. While waiting, publish loading <kind> on the status bar — the corvid busy indicator (Corvid-themed busy indicator: animated "bird at work" while long operations run (helm install, previews, drains) #143) animates it, so the wait reads as working.
  4. Run the unchanged push/pop+navigate transaction: _navigate_locked's start() is a no-op by then (watch already running → bucket not re-cleared), and the single post-switch render lands with real rows.

Edge handling

  • Live watch (split pane already showing the target kind/scope): skip both the restart and the wait — restarting would clear the bucket that pane is displaying.
  • Timeout (slow/unresponsive cluster, genuinely-empty child like an rs scaled to 0): degrade to exactly the old switch-then-fill behavior — never worse, at most the bounded wait.
  • Orphan reaping: _stop_watch_if_unused runs in a finally after the transaction — a drill whose pane closed while queued (or that raced a scope change) never leaks a stream; a landed navigation makes it a no-op.
  • NavigationStack.peek() added for the pop-side prewarm target; a pop that races a concurrent pop stops the wrongly-warmed watch via the same reaper.

Testing

New in tests/ui/test_drilldown.py (RED-first against a source with simulated LIST RTT, render spy asserts no (kind, 0) render ever happens):

  • push never renders an empty child view
  • pop never renders an empty parent view
  • progress label published while waiting, cleared once landed (drill driven as a task — pilot.press would await the whole transition)
  • timeout still switches (empty child view is correct; no wall-clock assertions — until() polling only)
  • a live watch from a split pane is reused, not restarted

test_concurrent_drill_and_navigate_stay_consistent now gates on the drill actually being blocked inside the critical section (event set in the stop() seam) instead of a 20ms sleep — the prewarm shifted the old timing assumption; the invariant it pins (no stranded drill state) is unchanged and still asserted.

Full gate green: ruff, mypy --strict, tach, 2946 passed / 21 skipped, coverage ≥ 80%.

Drilling deploy -> rs -> pods visibly happened in two steps: the pane
switched to a just-cleared bucket (WatchManager.start clears before the
LIST), rendered empty, and filled one network RTT later. Esc had the
same shape on the way back (the parent watch was stopped on the way
down, so navigating back re-cleared + re-LISTed).

_drill_into and _pop_drill now warm the target first: start the watch
while the current view is still up (a kind no pane displays renders
nowhere), wait - bounded by DRILL_PREWARM_TIMEOUT - until the rows the
transition will show exist (owned_by(parent_uid) for a push, any parent
row for a pop), then run the unchanged push/pop+navigate transaction.
_navigate_locked's start() is a no-op by then, the bucket is warm, and
the single post-switch render lands with real rows. While waiting the
status bar carries 'loading <kind>' - the corvid busy indicator (#143)
animates it.

- a live watch (split pane) skips both restart and wait
- timeout degrades to the old switch-then-fill, never worse
- _stop_watch_if_unused reaps the pre-warmed stream when the drill
  lost its pane or raced a scope change
- NavigationStack.peek() for the pop-side prewarm target

test_concurrent_drill_and_navigate_stay_consistent now gates on the
drill actually blocking inside the critical section instead of a
sleep - the prewarm shifted the old timing assumption; the invariant
it pins is unchanged.

Closes #157

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 2, 2026 15:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Pre-warms drill targets so push/pop transitions render populated views without empty-state flashes.

Changes:

  • Adds bounded watch pre-warming with progress feedback and cleanup.
  • Adds non-mutating navigation stack inspection.
  • Adds push, pop, timeout, progress, and split-pane tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/korvid/ui/app.py Implements drill target pre-warming.
src/korvid/ui/navigation.py Adds NavigationStack.peek().
tests/ui/test_drilldown.py Tests pre-warm behavior and rendering.
Suppressed comments (1)

src/korvid/ui/app.py:2301

  • The stack top is not guaranteed to still be peeked: pre-warming happens outside _nav_lock, so a later agent drill can push and navigate while this Escape is waiting. This code then pops that newly added level and navigates back, allowing the older pop to undo the newer action. Verify the top is still the captured level under the lock, or restart/abort the stale pop before mutating the stack.
                popped = pane.drill.pop()
                if popped is None:
                    return False
                await self._navigate_locked(pane, popped.parent_kind, None)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/korvid/ui/app.py
Review round 1 on #160: the pre-warm widened the window between the
drill's intent and its locked transaction to up to a second, so a newer
:view/:ns or a context switch landing meanwhile could be overridden by
the stale drill (or worse, an old-cluster UID applied after the epoch
changed). Both _drill_into and _pop_drill now anchor (kind, scope) and
_ctx_epoch before the pre-warm and revalidate under the lock - a
mismatch abandons the drill with an accurate result string (never a
false 'drilled into ...' success for the agent), and the pop side also
requires the peeked level to still be the top of the stack. Abandoned
streams are reaped by the existing _stop_watch_if_unused finally.

Tests: test_drill_abandons_when_a_newer_navigation_lands_during_prewarm,
test_drill_abandons_across_a_context_epoch_change,
test_pop_abandons_when_the_view_changed_during_prewarm.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/korvid/ui/app.py:2274

  • This stale check misses a newer explicit navigation to the same (kind, scope). on_navigate_command still represents a later command and clears drill state, but because the tuple and context epoch remain unchanged, the older prewarm then pushes its level and overrides that command. Track and revalidate a per-pane navigation/request generation that advances even for same-target navigations.
                if (
                    (pane.kind, pane.scope) != origin
                    or self._ctx_switching
                    or epoch != self._ctx_epoch
                ):

src/korvid/ui/app.py:2269

  • Returning None denotes success to agent_drill_down, so if the initiating pane is closed during the newly added prewarm window the agent reports that it drilled and records a breadcrumb even though no navigation occurred. Return an abandonment error here so callers receive an accurate outcome.
                if pane not in self._panes:
                    return None  # the initiating pane was closed while queued

src/korvid/ui/app.py:2201

  • active also includes a watch started by another in-flight prewarm, not only a watch serving a pane. If an agent drill and keyboard drill overlap, the second call returns here before the LIST lands, switches to the target, and recreates the empty-view flash. Cleanup can also stop that shared watch when one drill abandons. Distinguish pane-backed watches from prewarm leases (for example, wait on each caller's readiness predicate and reference-count leases so only the last unused lease stops the watch).

This issue also appears in the following locations of the same file:

  • line 2268
  • line 2270
        if (kind, scope) in self.watch_manager.active:
            return
        await self.watch_manager.start(kind, scope)

Review round 2 on #160 (all three suppressed findings credible):

- a ':view deployments' issued while already on deployments is still
  the newer command (it clears drill state) but left the (kind, scope)
  tuple and epoch unchanged - the stale drill pushed over it. Every
  _navigate_locked call now advances a per-pane nav_gen, and both drill
  paths capture + revalidate it under the lock
  (test_drill_abandons_when_a_same_target_navigation_lands_during_prewarm).
- a pane closed during the pre-warm returned None, which
  agent_drill_down reports as a successful drill with a breadcrumb -
  now an accurate abandonment string
  (test_pane_closed_during_prewarm_reports_abandonment).
- watch_manager.active includes another drill's in-flight pre-warm, so
  an overlapping drill skipped its wait and recreated the empty flash,
  and one drill's cleanup could stop the stream the other relied on.
  _prewarm_view skips only pane-backed watches, always waits on the
  caller's own readiness, and _stop_watch_if_unused is lease-counted:
  only the last release may reap an undisplayed stream
  (test_overlapping_drills_do_not_skip_each_others_prewarm).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@hellices

hellices commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Round 2's three suppressed findings were all credible and are fixed:

  • same-target navigation invisible to the tuple check — every _navigate_locked call advances a per-pane nav_gen (including same-target ones); both drill paths capture and revalidate it under the lock. Test: test_drill_abandons_when_a_same_target_navigation_lands_during_prewarm.
  • pane closed during pre-warm → false agent success — that branch now returns an accurate abandonment string instead of None. Test: test_pane_closed_during_prewarm_reports_abandonment.
  • in-flight pre-warm mistaken for a warm watch_prewarm_view skips only pane-backed watches and always waits on the caller's own readiness predicate; _stop_watch_if_unused is lease-counted so only the last release reaps an undisplayed stream. Test: test_overlapping_drills_do_not_skip_each_others_prewarm (render spy asserts neither drill flashes empty).

Full gate green (2952 passed).

@my-reviewer-agent my-reviewer-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이슈 #157 드릴 프리웜 리뷰. 설계가 탄탄합니다: (1) 스위치 전에 타깃 watch를 시작해 LIST가 보이지 않는 곳에서 수행되고, 렌더 스파이 테스트가 (kind, 0) 렌더가 한 번도 없음을 직접 고정. (2) 프리웜 대기(최대 1s)로 넓어진 경합 창을 origin+nav_gen+_ctx_switching+epoch 4중 앵커로 락 안에서 재검증 — 특히 같은 (kind,scope)로의 :view 재진입까지 잡는 per-pane nav_gen 카운터와 그 전용 테스트가 좋습니다. (3) pop 쪽은 peek() identity 비교까지 더해 stale pop이 네비게이션을 덮어쓰지 않고 Esc만 소비(True). (4) _prewarm_leases 카운트로 겹치는 드릴이 서로의 프리웜을 warm으로 오인하지 않게 하고(재현 테스트 포함), finally의 _stop_watch_if_unused가 마지막 lease에서만 pane 미표시 스트림을 회수. (5) pane 닫힘 시 None 대신 정확한 abandonment 문자열을 반환해 agent_drill_down의 거짓 성공 보고를 막은 것, 기존 동시성 테스트의 20ms sleep을 entered 이벤트 게이트로 바꾼 것 모두 올바른 방향입니다. 타임아웃 시 기존 switch-then-fill로 정확히 퇴행하는 것도 확인. 인라인으로 예외/취소 경로의 lease·watch 누수 가능성 1건만 남깁니다 (정상 경로에는 영향 없음).

Comment thread src/korvid/ui/app.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/korvid/ui/app.py:2222

  • The pane-backed fast path races watch teardown. _navigate_locked leaves pane.kind/scope unchanged while awaiting WatchManager.stop(), but stop() removes the key from active before that await. A concurrent drill can therefore see the target pane here and skip both start() and the wait; once the other navigation completes, the target watch is gone, so the drill's later start() clears the bucket and the empty-view flash returns. The lease also is not consulted by _navigate_locked or pane-close watch stops. Please synchronize this check with watch lifecycle, or require an active watch and make teardown honor outstanding pre-warm leases.
        self._prewarm_leases[key] = self._prewarm_leases.get(key, 0) + 1
        if any((p.kind, p.scope) == key for p in self._panes):
            return

src/korvid/ui/app.py:2348

  • This readiness test is insufficient when popping the second level of deployments → replicasets → pods. After the pop, the remaining drill level filters replicasets by the deployment UID (_render_pane lines 1297-1299), but bool(rows) can become true for an unrelated ReplicaSet first; the switch then renders zero visible rows and flashes empty until an owned row arrives. Build the readiness predicate from a copied stack after one pop, and wait for a row owned by its remaining parent_uid (falling back to bool(rows) only when the pop returns to the root).
        await self._prewarm_view(peeked.parent_kind, prewarm_scope, lambda rows: bool(rows))

Review round 3 on #160:

- the lease acquire sat outside the try: a drill task cancelled
  mid-pre-warm (:ctx teardown) left a permanent lease - blocking every
  later reap on that (kind, scope) - and leaked the started watch. The
  pre-warm call moved inside the try; the acquire is synchronous before
  the first await, so the finally never releases a lease that was not
  taken (test_cancelled_prewarm_releases_its_lease_and_watch).
- the pane-backed fast path raced watch teardown: a pane mid-navigate
  keeps its tuple while stop() has already removed the stream. The fast
  path now also requires the watch to be live, and _navigate_locked's
  teardown skips streams with outstanding pre-warm leases - the last
  lease release reaps them
  (test_prewarm_restarts_a_dead_watch_even_when_pane_backed,
  test_navigation_teardown_honors_outstanding_prewarm_leases).
- popping pods -> replicasets keeps the deployment-UID filter, but the
  readiness accepted any row: an unrelated ReplicaSet arriving first
  switched into a zero-row filtered view. Readiness now mirrors what
  the post-pop view will show - owned rows for a remaining level, any
  row only at the root
  (test_two_level_pop_waits_for_rows_the_drill_filter_will_show).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/korvid/ui/app.py:2224

  • This lease is not honored by _close_focused_pane, which unconditionally stops the closing pane's distinct watch at app.py:4498-4499. If a drill targets a live split pane while that pane is being closed (the close can yield while shutting down its log pane), pre-warm observes the still-active rows and returns, then the close kills the leased watch; _navigate_locked restarts it and clears the bucket, recreating the empty flash. Make pane-close teardown use the same lease check as navigation, leaving the final lease release to reap the watch.
        self._prewarm_leases[key] = self._prewarm_leases.get(key, 0) + 1

@my-reviewer-agent my-reviewer-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

재리뷰 (신규 커밋 1개, cc144e6f1b4ad4) — 라운드 3의 inline Warning이 정확히 해결되었고, 추가로 두 가지 레이스까지 정리되었습니다.

1. 리스 누수 해결 (이전 Warning)_prewarm_view await가 try/finally 안으로 이동했습니다. 리스 획득은 첫 await 이전의 동기 구간이므로 "획득하지 않은 리스를 finally가 해제하는" 역누수도 구조적으로 불가능합니다. test_cancelled_prewarm_releases_its_lease_and_watch가 취소 후 리스 소멸 + watch 미잔존을 실제로 검증합니다.

2. pane-backed fast path의 teardown 레이스 — 판이 튜플을 유지한 채 stream이 이미 stop()된 경우를 key in self.watch_manager.active 조건 추가로 막았고, 대칭으로 _navigate_locked의 teardown이 outstanding 리스가 있는 stream을 건너뛰도록 하여(마지막 리스 해제가 reap) fast path와 teardown 양쪽이 일관됩니다. 두 방향 모두 테스트로 고정 (test_prewarm_restarts_a_dead_watch_even_when_pane_backed, test_navigation_teardown_honors_outstanding_prewarm_leases).

3. 2단계 pop의 readiness 정합성 — pods→replicasets pop은 deployment-UID 필터를 유지하는데, 기존 readiness는 임의 행을 수락해 무관한 ReplicaSet이 먼저 도착하면 0행 필터 뷰로 전환될 수 있었습니다. 이제 under = pane.drill.copy(); under.pop()으로 pop 이후 뷰가 실제로 보여줄 조건(잔여 레벨이면 owned 행, 루트면 임의 행)을 그대로 미러링합니다. 스파이 렌더 테스트가 ("replicasets", 0) 렌더 부재를 고정합니다.

지적사항 없음. APPROVE

@hellices
hellices merged commit d678652 into main Aug 2, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drill-down transitions flash an empty view before rows arrive — pre-warm the target watch

2 participants