[ShryukGrandhi]

What and why

test_a_silent_model_gets_stuck_and_cancel_ends_the_run failed on the Windows engine tests of #406 with assert 0 == 1 on its last line, llm.stream_calls == 1.

The test set tasks.stuck_after_minutes = 0.01, which is about 0.6 s of real time. The executor starts the no-activity timer (stall = asyncio.timeout(...) in tasks/executor.py) before it builds the system prompt, recalls memories and saves the first transcript. On a busy runner that setup can take longer than 0.6 s. When it does, the run is marked stuck before the model is ever called. The reason text still matches ("the AI model hasn't answered"), so every assertion passes except the final call count, which is 0.

The fix follows the other no-activity tests in the file, which already skip time with skip_clock instead of relying on real time. The silent model now moves the clock past a 30 s limit only once it has actually been called, then stays silent. To time out early now, setup would need 30 s of real time, and the outcome no longer depends on how fast the machine is. This is a test-only change, so the engine's behaviour stays the same, and no flaky marker was needed.

Fixes #438

How I tested it

• Reproduced the CI failure locally by making _executor_system_prompt sleep 1 s before building the prompt (a pytest plugin outside the repo, not committed). Before the fix the test fails with assert 0 == 1, exactly as on CI. After the fix it passes with that 1 s delay, and also with a 5 s delay.
• pytest -q tests/tasks/test_stuck_runs.py passes 8/8, run 5 times in a row.
• pytest -q tests/tasks passes (374 tests).
• ruff check sentient tests packaging is clean.
• Full pytest -q run: 2740 passed, 7 failed. The 7 failures are in voice, Docker sandbox, Claude Code and hardware-detection tests, none of which this change touches. When I re-ran them on the unmodified test file, six failed the same way and the seventh passed, so none of them come from this change.

Real test

No model involved: this only changes a unit test's timing. I checked it against the executor code it exercises (the stall timer in tasks/executor.py) and forced a slow setup to reproduce the CI failure.

Checklist

• Tests added or updated, and pytest / npm run typecheck pass locally for what I touched
• ruff check sentient tests is clean
• docs/API.md updated if the desktop ↔ engine contract changed (not changed)
• A changelog entry in changes/ if users will notice this change (test only, not needed)
• No keys, tokens or personal data in the code, tests or screenshots

Summary by CodeRabbit

• Tests
  • Updated stuck-run coverage to account for model startup latency.
  • Adjusted expectations for inactivity detection to use the 0.5-minute threshold.
  • Preserved existing cancellation behavior checks.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 73269681-d00d-4414-9173-381f6a7a17a6

📥 Commits

Reviewing files that changed from the base of the PR and between ee7a684 and 350250d.

📒 Files selected for processing: 1

• tests/tasks/test_stuck_runs.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The silent-model stuck-run test now delays model output with SilentProvider and advances time with SkipClock. It checks the shared 0.5-minute inactivity threshold instead of using a real-time 0.01-minute timeout.

Changes: Stuck-run test timing

Layer / File(s)	Summary
Silent-model timeout test tests/tasks/test_stuck_runs.py	The test uses SilentProvider and SkipClock to check the 0.5-minute inactivity threshold. The expected stuck reason is updated; cancellation assertions remain unchanged.

Priority: ⬇️ Low

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

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 35025

This test-only change removes setup-time timing sensitivity and is suitable to merge.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: removing setup-time dependence from the silent-model stuck test.
Linked Issues check		Issue #438 requires fixing the timing assumption in test_a_silent_model_gets_stuck_and_cancel_ends_the_run, or marking the test as flaky. The diff removes the 0.01-minute wall-clock limit, injects `…
Out of Scope Changes check		The diff changes only tests/tasks/test_stuck_runs.py. The added provider and clock-controlled test behavior directly support issue #438. The changes do not modify engine behavior or add unrelated fi…

Full details: Linked Issues check

Explanation

Issue #438 requires fixing the timing assumption in test_a_silent_model_gets_stuck_and_cancel_ends_the_run, or marking the test as flaky. The diff removes the 0.01-minute wall-clock limit, injects skip_clock, and advances the clock only inside SilentProvider.stream, after the model call starts. The test keeps the call-count assertion and verifies cancellation. This removes setup-time dependence without changing engine code.

Full details: Out of Scope Changes check

Explanation

The diff changes only tests/tasks/test_stuck_runs.py. The added provider and clock-controlled test behavior directly support issue #438. The changes do not modify engine behavior or add unrelated files.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks the clock,
The model waits before it speaks,
Half a minute passes,
The stuck-run test records the pause,
Then cancellation ends the run.}

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

[itsskofficial]

Thanks @ShryukGrandhi, nice diagnosis! Reproducing it with a slowed-down setup was exactly the right move. Locally, main failed 2 of 45 runs next to the full suite. Your version passed all 45 there, plus 50 quiet runs and 64 runs at 16 at once. Moving the time jump to after the model is called makes the test independent of setup time instead of just stretching a deadline, and it's about 4x faster too. Approving and merging.