fix(ask): require fresh response in completion branches#19
Conversation
…n answers)
Each completion branch in `comet_ask` returned `status.response`
whenever it looked plausible:
- explicit `status.status === 'completed'` + saw-new-response
- `status.isStable` + no stop button
- idle timeout > 6s + substantial response
What's missing in all three is "this response is actually the answer
to the prompt we just sent". `extractAgentStatus`, even after the
`lastIndexOf` fix in the companion correctness PR, can still pick up
the PREVIOUS turn's answer when:
- the new prompt is sent into the same chat tab
- Perplexity has not yet visibly overwritten the prose / steps marker
- the stop-button hasn't appeared yet (slow render)
- `sawNewResponse` is already true because a prose count went up
briefly during the "Thinking…" placeholder render
Each branch therefore happily returned the prior turn's text as the
answer to the new question. Silent wrong-answer — the single most
dangerous correctness defect in the polling logic.
Fix: snapshot the full `extractAgentStatus().response` BEFORE sending
the prompt, and require `status.response !== oldResponseSnapshot` for
all three completion branches plus the max-timeout fallback. If
everything we see is still the old answer, fall through to the
"in progress" branch and let the caller poll again.
`oldResponseSnapshot` is captured in a `try { … } catch { }` so a
failing pre-send status check doesn't break the prompt flow — the
guard simply degrades to "any non-empty response is fresh".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Confirming the bug from an independent repro and adding context on a complementary fix. I hit this during a long multi-turn agentic-browsing session, polling at ~30-90s cadence between batches. Symptom matched the diagnosis here exactly: The pre-send snapshot approach here is a clean defense at the return path and catches the failure regardless of which extraction strategy fires. Worth knowing about: I shipped two complementary local fixes that sit at different layers, so they compose rather than compete with this PR.
One edge case worth flagging on this PR's approach: if a user asks the same question twice and the page-derived answer is byte-identical to the previous turn (terse yes/no, canonical short answer), Both commits sit on my fork at the SHAs above. I was planning to open PRs for those, but you're welcome to absorb into this PR if that's easier. |
Code Review: fix(ask): require fresh response in completion branchesReviewed commit: Overall Assessment: APPROVE with minor observationsThis is a well-reasoned fix for a real and dangerous bug class — returning the previous turn's answer as if it were the current one. The approach is sound: snapshot the full response text before sending the prompt, then gate all completion branches on Stale Answer Detection LogicCorrectness: Good. The core invariant is clear and correctly applied: a response that is byte-for-byte identical to the pre-prompt snapshot cannot be the answer to the new prompt. The const responseIsFresh = !!status.response && status.response !== oldResponseSnapshot;This correctly handles:
One edge case to be aware of (non-blocking): If Perplexity returns the exact same text for two different questions (e.g., "What is 2+2?" asked twice), the freshness check would incorrectly classify the second answer as stale. However, this is extremely unlikely in practice since Perplexity responses include timestamps, source citations, and formatting that vary between invocations. The fix correctly trades this theoretical false-negative for eliminating a real class of silent wrong-answers. Acceptable tradeoff. Race ConditionsNo new race conditions introduced. Existing ones mitigated.
Completion Branch CorrectnessAll three completion branches plus the max-timeout fallback consistently apply
The substitution is consistent — Minor Observations (non-blocking)
VerdictAPPROVED. The fix is correct, minimal, and handles failure modes gracefully. The stale-answer detection is sound, no race conditions are introduced, and all completion branches are consistently guarded. The degradation path (snapshot capture fails) preserves pre-fix behavior. Safe to merge. |
Summary
Each completion branch in `comet_ask` returned `status.response` whenever it looked plausible:
What's missing in all three is "this response is actually the answer to the prompt we just sent". `extractAgentStatus` can still pick up the previous turn's answer when:
Each branch then returns the prior turn's text as the answer to the new question. Silent wrong-answer — the single most dangerous correctness defect in the polling logic.
Fix
Snapshot the full `extractAgentStatus().response` BEFORE sending the prompt, and require `status.response !== oldResponseSnapshot` in all three completion branches plus the max-timeout fallback. If everything we see is still the old answer, fall through to the "in progress" branch and let the caller poll again.
`oldResponseSnapshot` is captured in a `try { … } catch { }` so a failing pre-send status check doesn't break the prompt flow — the guard simply degrades to "any non-empty response is fresh".
Compatibility
No public API change. `comet_ask` returns the same shape; it just no longer returns previous-turn answers. Worst case (snapshot capture fails) is identical to current behaviour.
Test plan
🤖 Generated with Claude Code