fix(progress): name the running operation in watchdog status lines#3443
Conversation
The watchdog status named the awaited artifact ("Waiting for the commit
message"), which reads like waiting on user input. Name the running
operation instead: the LLM sites say "the commit generation command"
(matching the [commit.generation] command key and the "Commit generation
command failed" error title), the version check says "the version
check", and pr:/mr: resolution says "the PR/MR lookup".
All three LLM sites run the same configured command, so the phrase moves
into watch_llm_command and its waiting_for parameter is dropped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
Wording change reads cleanly — the watchdog now names the running operation ("Waiting for the PR lookup", "the version check", "the commit generation command") rather than the awaited output, and the doc comments plus every test assertion are kept in sync. The watch_llm_command signature drop is reflected at all three call sites. No stale wording remains anywhere in the tree. Nothing to change in the code.
Not approving only because codecov/patch is red — and it's worth noting why, since it's a false alarm for this PR. The single uncovered line is src/commands/config/show.rs:1518, the string-only edit inside fetch_latest_version(). That line was already uncovered before this PR — it lives on the curl → api.github.com path that tests don't exercise by design — and codecov re-flags it as a new patch line purely because the literal changed. The other three touched files (progress.rs, llm.rs, switch.rs) are 100% covered.
There's no meaningful test to add here (covering it would mean mocking the version-check network call for a string swap), so this is an acceptable pre-existing gap. Per CLAUDE.md, merging past a red codecov/patch is your call.
The guard scoped to the curl block sat in the network-only path that tests short-circuit via WORKTRUNK_TEST_LATEST_VERSION, so the reworded label read as an uncovered patch line. Held from function entry it behaves identically (nothing renders inside the 2s startup delay) and executes under the existing config-show tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
codecov/patch failed on HEAD, and this commit is the cause — it made the gap larger, not smaller. Dismissing my earlier approval accordingly.
The reword goal succeeded: moving Watchdog::start to function entry (line 1500) does get it covered by the existing config-show tests. But lifting it out of the let output = { … } block re-indented the whole curl call from 8 spaces to 4, so Codecov's line-diff now treats all 13 curl-block lines as newly added. Those lines live on the api.github.com path that tests short-circuit via WORKTRUNK_TEST_LATEST_VERSION, so they're uncovered — patch coverage for show.rs is 7.14% (1 hit / 14 lines). Before this commit the patch had exactly one uncovered line (the reworded label); now it has thirteen.
Two ways out:
-
Keep the block wrapper (recommended — gets
codecov/patchgreen). Leave the watchdog at the top where it is now, but keeplet output = { Cmd::new("curl")…run()? };as a block so the curl lines stay at their original indentation. Codecov then sees only the watchdog line in the diff (covered), and the never-tested curl lines drop out of the patch entirely since their bytes are unchanged from the base:let _watchdog = worktrunk::progress::Watchdog::start("the version check", None); // … let output = { Cmd::new("curl") .args([ // …unchanged… ]) .run()? };
-
Revert this commit. That returns to the single-line gap my prior review already judged an acceptable pre-existing gap (the network path is untestable by design), which per
CLAUDE.mdis a redcodecov/patchyou can merge past at your discretion. Simpler, but loses the coverage the reword gained.
Happy to push either — say the word and I'll open a follow-up commit on this branch. Everything else in the PR (the wording change and its tests) is unaffected and reads cleanly.
codecov/patch failed — this commit re-indented the untested curl block into the patch, taking uncovered added lines from 1 to 13. See follow-up review.
Restores the braces around the curl call so its lines keep their original indentation: the watchdog hoist re-indented them, and codecov counted all 13 never-tested network-path lines as new patch lines. With the bytes unchanged from the base they drop out of the patch; the hoisted watchdog line remains the only coverable addition, exercised by the config-show tests. Suggested by the tend review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The watchdog status line named the awaited artifact —
↳ Waiting for the commit message (4s)— which reads like waiting on user input. It now names the running operation:wt config showconnectivity test):Waiting for the commit generation command (4s), reusing the existing vocabulary ([commit.generation] commandconfig key, "Commit generation command failed" error title). All three sites run the same configured command, so the phrase moves intowatch_llm_commandand itswaiting_forparameter is dropped.wt config show --full:Waiting for the version check (4s)wt switch pr:N/mr:N:Waiting for the PR lookup (4s)/the MR lookupThe escalated form now reads coherently, with the status naming a command and the gutter revealing it:
The version-check watchdog guard now spans the whole
fetch_latest_versionlookup rather than just the curl block; nothing renders inside its 2s startup delay, so behavior is unchanged, and the line executes under the existing config-show tests (codecov/patch flagged it as an uncovered patch line in the network-only path).