chore(streaming): refresh reasoning-only watchdog on current main - #2
Draft
enzo-adami wants to merge 4 commits into
Draft
Conversation
…NousResearch#78807) Models like DeepSeek V4 Flash can emit reasoning_content tokens indefinitely without ever committing to visible output. Every reasoning chunk resets last_chunk_time, so the existing stale detector never fires and the session hangs until the HTTP timeout (up to 1800s). Add a second timer last_content_chunk_time that resets only on real output (content text or tool calls), plus a reasoning_seen flag so the new check only activates once the model has started reasoning (slow first-token models stay under the existing _stream_stale_timeout guard). When reasoning-only output exceeds agent.reasoning_only_stale_timeout (config.yaml, default 300s, 0 disables), the poll loop cancels the attempt and aborts the request client so the retry loop reconnects. Config-driven per repo policy (no new HERMES_* env var; the previous attempt NousResearch#29796 was closed for introducing one). Covers both the chat-completions (reasoning_content) and Anthropic (thinking_delta) streaming paths. Tests drive the real interruptible_streaming_api_call with fake chunk streams: kill after threshold, config read from config.yaml, disable at 0, no false positive on reasoning-then-content or content-only streams.
Review findings (NousResearch#78807, cross-vendor): - Anchor last_content_chunk_time at the FIRST reasoning chunk, not the attempt start, so TTFT latency does not eat the reasoning budget. - Track input_json_delta in the Anthropic path: long tool-argument streams after reasoning must keep the watchdog satisfied. - Reject bool config values (bool is an int subclass); 'true' would otherwise become a 1-second kill. - Floor the unconfigured default at the model's reasoning stale floor (e.g. deepseek-v4-flash 600s) so the detector never fires before the no-chunk detector's established tolerance; explicit config wins. - Do not retry after a reasoning-only kill: the loop is prompt-deterministic, a byte-identical retry would be killed again; the outer conversation loop's fallback machinery recovers instead.
Cross-vendor round-2 finding: the configured flag was set for ANY non-None value, so a malformed value (bool, string) marked the key as configured and silently bypassed the model's reasoning stale floor. Only mark the key configured when the value is a valid number; log a warning otherwise so the floor-aware default still applies.
DavidMetcalfe
force-pushed
the
fix/78807-reasoning-only-stale-detector
branch
from
August 16, 2026 23:04
ddca016 to
7271f8d
Compare
DavidMetcalfe
pushed a commit
that referenced
this pull request
Aug 18, 2026
Two independent bugs let a deleted profile reappear / leave orphaned resources on next launch: 1. hermes_cli/profiles.py's backend-process scanner required argv[0] to resolve to an executable literally named "hermes". Electron's pool-backend spawn resolves the hermes console-script shim's path and execs it via the interpreter directly (python3 /path/to/hermes ...), so argv[0] reports as "python3" and the scanner never matched the running backend -- delete removed the profile's files but left its live backend process running (still bound to a port via uvicorn), which accumulates across repeated delete/recreate cycles. 2. The desktop sidebar's ProfileRail only refreshed its cached profile list once, on mount, so a delete/create/rename from another surface (another window, or the CLI) left a stale ghost entry until something unrelated triggered a refetch. Note: a delete via this window's own Manage-Profiles view already refreshes the shared $profiles atom ProfileRail subscribes to (confirmed by reading refreshProfiles() and handleConfirmDelete()) -- this fix only covers the cross-window/cross- process staleness gap, not a duplicate of the already-merged NousResearch#57329's Manage-Profiles rail-refresh work. Fix 1: recognize a python-interpreter argv[0] exec'ing a hermes-named console-script shim via argv[1]. Fix 2: refresh the profile list on window focus/visibilitychange, matching the existing pattern used elsewhere in the sidebar (sidebar/index.tsx, use-background-sync.ts, star-map.tsx, use-gateway-boot.ts all use the same focus+visibilitychange pattern). ## Related work already on main PR NousResearch#57329 (merged) fixed the *headline* symptom from issue NousResearch#52279 (deleted profile respawns) via a different, non-overlapping mechanism: routing profile-delete through the primary backend instead of spawning a fresh pool backend, plus a separate recreation guard in ensure_hermes_home() (NousResearch#49435, merged) that makes a backend spawned into a deleted profile's directory raise FileNotFoundError instead of silently recreating it. This PR is NOT a duplicate of that fix. Verified: even with both of those merged, a backend process that survives because of gap #1 above still holds a bound port via uvicorn -- it just can no longer resurrect the profile directory. That's real resource-hygiene, not a symptom already covered. Gap #2 touches a different file/component (ProfileRail / profile-switcher.tsx) than NousResearch#57329's rail-refresh half (which touched the Manage-Profiles view's own $profiles.ts / index.tsx) and covers a distinct staleness path (cross-window/cross-process, not same-window delete-then-refresh). Tests: tests/hermes_cli/test_profiles.py -- 156 passed (existing + regression coverage for the argv[0] python-interpreter detection case). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DavidMetcalfe
added a commit
that referenced
this pull request
Aug 18, 2026
Addresses teknium1's review (NousResearch#64195) finding #2: the multi-rung resolver needs Electron tests covering precedence, stale-PID rejection, fallback behavior, and the remote boot path. The pure decision helpers are now covered by 29 unit tests in `profile-migration.test.ts` (vitest electron project). Coverage: - precedence: legacy > single-running-gateway > state.db heuristic - stale-PID rejection: recycled PIDs not owned by hermes are dropped - malformed pid files: JSON parse errors, non-integer PIDs, zero/negative - scoring edge cases: ancient files (recency floored at 0.1), tiny files (size floored at MIN_SIZE), larger DB beats smaller at similar recency - single-profile fallback: best === 'default' suppresses the write - no-op cases: preference file already exists, missing profiles root The remote boot path is verified by code review of the call-site move (commit preceding this one) — `migrateActiveProfileIfMissing()` now runs before `primaryProfileKey()` is first read in `startHermes()`. The pure decision logic that the orchestrator relies on is covered end- to-end below; this matches the repo's testable-helper pattern (see `profile-delete-routing.test.ts`).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Cross-fork handoff for NousResearch/hermes-agent#81526. This refreshes DavidMetcalfe's three watchdog commits onto upstream
mainat165c889e5b4277b56dadd42949a4112c1e6175a6; it is not a replacement PR against NousResearch.Important
This is a review/branch handoff for a rebased history. GitHub therefore reports it as conflicting against the pre-rebase source branch; it is not intended to be merged mechanically into that old history. The author can inspect it and force-update the source branch to this tested head (or cherry-pick the final oracle commit after performing the same semantic rebase).
Semantic rebase
The only conflict was in
agent/chat_completion_helpers.pyat the tool-call boundary. The resolution preserves both current-main pending-SSE behavior and the watchdog's progress timestamp, in this order:The three original commits and authorship are preserved. The only additional commit adds two focused regression oracles:
Verification
42 passed, 4 skippedacrosstests/agent/test_reasoning_stale.pyandtests/run_agent/test_streaming.pygit diff --check: passThe source branch was still exactly
ddca016590eeadc94b3d4840e2681a9316fdd33fwhen this handoff was pushed.