Reset the progress display between top-level executions

[ahogappa]

Problem

Surfaced by the #240 adversarial review. The progress display is a persistent singleton (Config#build memoizes @cached_display), reused across sequential top-level executions in one process. The displayed root was never reset and @tasks accumulated — so a second top-level Task.run / run_and_clean rendered the first execution's root name and an accumulated task count:

[TASKI] Starting FirstRoot      # run #1
[TASKI] Completed: 1/1 tasks
[TASKI] Starting FirstRoot      # run #2: stale root — should be SecondRoot
[TASKI] Completed: 2/2 tasks    # accumulated — should be 1/1

Pre-existing; hits any program running ≥2 top-level tasks per process — scripts, test suites, REPLs.

Fix

Clear per-execution state when the outermost execution stops (on_stop at @nest_level == 0), after the final frame has rendered and handle_stop has stopped the render thread. The next top-level execution then sees @root_task_class == nil and adopts its own root/capture via the normal on_ready path; nested executions stop at @nest_level > 0 and never trigger the reset.

Why reset at stop, not lazily at the next on_ready: run_and_clean calls notify_start before its first on_ready. A reset deferred to on_ready would come too late for the start line and mis-detect the new execution as nested. Resetting at stop sidesteps the ordering and leaves on_ready unchanged from #240. (An earlier draft keyed the reset off @nest_level in on_ready and was caught during self-review failing exactly the run_and_clean×2 case — hence the stop-based approach, pinned by a dedicated test.)

reset_after_execution (run from an ensure, so a raising final render still clears state — dispatch swallows observer errors) clears the base tallies (@tasks, @group_start_times, @start_time), @root_task_class / @display_facade / @output_capture, and @spinner_index, then calls a handle_reset hook; Tree re-inits its node maps, Tree::Live also drops @last_line_count, and Simple clears its group baselines.

Tests

• End-to-end through the real executor: two sequential plain runs and two sequential run_and_clean (each shows its own root and 1/1, no accumulation); singleton-reused guard; state-cleared-after-stop.
• Component: on_stop clears per-execution state incl. spinner index; reset-via-ensure when the final render raises; a second execution adopts its own state (incl. readying at a raised nest level); the Tree node map is rebuilt for a second execution; Tree::Live's @last_line_count is reset.
• Existing once-guard tests updated to simulate real nesting.

Suite 856 runs / 0 failures, rake standard clean. Off master, independent.

Adversarial review

A fresh 15-agent review of the reset-on-stop design (the earlier review of a discarded @nest_level draft was stopped). 4 confirmed findings, all folded: spinner index not reset (LOW); reset skipped if the final render raises → moved into an ensure (MED); two coverage gaps (no two-run Tree-node test, no Tree::Live @last_line_count test) → tests added and mutation-verified. Refuted: post-run task_state returning nil is an intentional, accepted semantics change; @context retaining a stale facade is informational; nested/clean unaffected.

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

Refines Layout::Base#on_ready to distinguish same-facade re-readying from new top-level executions, introducing reset_display_state and a handle_reset hook. Subclasses Simple::Display and Tree::Structure implement the hook to clear their per-execution state. New tests and fixtures verify non-accumulation across sequential top-level runs.

Changes

Per-execution display state reset

Layer / File(s)	Summary
on_ready branching and reset_display_state hook lib/taski/progress/layout/base.rb	on_ready now early-returns for same-facade re-readying (only re-adopts @output_capture), and only calls reset_display_state when a different facade signals at nest level zero. reset_display_state clears @tasks, @group_start_times, and start time, then calls the handle_reset no-op hook.
handle_reset in subclasses lib/taski/progress/layout/simple.rb, lib/taski/progress/layout/tree/structure.rb	Simple::Display#handle_reset clears @group_baselines; Tree::Structure#handle_reset re-runs init_tree_structure to reset @tree_nodes and @node_prefixes; both call super.
Layout::Base unit tests test/test_layout_base.rb	Replaces the single "set root only once" test with explicit nested-execution and new-top-level-execution cases; refines the capture retargeting test to add on_start before a nested on_ready.
Integration tests and fixtures test/fixtures/display_reset_tasks.rb, test/test_display_reset_between_executions.rb, test/test_simple_progress_display.rb	Adds DisplayResetFixtures with two independent roots; adds integration tests asserting no cross-execution task accumulation and singleton display reuse; updates TestSimpleProgressDisplay with nested and top-level on_ready registration tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• ahogappa/taski#240: Directly modifies the same Layout::Base#on_ready method and @output_capture adoption logic, with overlapping test/fixture changes around facade identity.
• ahogappa/taski#57: Introduced the output-capture plumbing that on_ready now conditionally re-adopts per the same-facade vs. new-root branching added in this PR.

Poem

🐇 Hop, hop — the display forgets the old run,
Each root gets its own fresh state under the sun.
handle_reset sweeps the baselines away,
The tree shakes its nodes at the start of the day.
No stale task count lingers, no ghost from before —
The singleton stays, but its memories? No more! 🌱

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and clearly summarizes the main change: resetting the progress display between sequential top-level task executions to fix the bug where state was accumulated across runs.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/display-reset-between-executions

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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