[devswha]

Fixes #2654

Change

scripts/chat-browser-qa.ts:121 (await work.click()) failed Playwright strict mode on main: log.locator(".work-block-head") matched both the settled "Worked for 7s" head and a div.work-block-head reading "Working…". Test-only: no product code changes.

The second head is the footer's PendingWorkHead (src/components/ChatView.tsx:2378-2385). The fixture's answer lands while the pane still reads working, so the chat keeps the next turn's Working head after that answer (afterAnswer, hidden as chat-activity-reserved for the first 800 ms, then shown as chat-activity) until the idle status arrives. The script reported idle and waited for the settled head, but not for this footer head to go.

That head is intended behaviour (see the comments at ChatView.tsx:2176-2192), so the fix is in the script. Before the click, it now waits for .chat-activity, .chat-activity-reserved to be detached. report("idle") has already been sent, nothing reports working before the click, and Playwright's 10 s default bounds the wait.

Validation

• Failing first: on main 59e51d0, bun run build && bun scripts/chat-browser-qa.ts failed with the issue's strict-mode error at line 121.
• Fixed: bun scripts/chat-browser-qa.ts printed all 9 PASS lines and exited 0 in 2 of 2 runs, under dori heavy.
• bun run check fast: success on this branch.
• omo code reviewer: APPROVE, no findings; it also ran the script twice (pass).
• Codex Terra (gpt-6.1-terra, xhigh): APPROVE.
• Skipped: full check, Windows VM. This script is not in CI (scripts/ci-browser.test.ts lists it as not run).

No changelog fragment: test-only.

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Tests
  • Updated browser QA checks to wait until chat activity has fully settled before verifying completed work blocks, improving the consistency of test results.

Changes

The browser QA script waits for .chat-activity and .chat-activity-reserved to detach after the pane reports idle. It then continues to the settled work-block check.

Chat browser QA

Layer / File(s)	Summary
Wait for activity indicators scripts/chat-browser-qa.ts	After the answer appears and the pane reports idle, the test waits for both activity selectors to detach before continuing. This prevents the transient “Working…” head from remaining alongside the settled work-block head during the check.

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

Change: Other · Severity of issue fixed: Low

Possibly related PRs

• devswha/ovis#2551: Changes when Chat renders the pending Working head and adds QA coverage for a finished turn followed by new work.
• devswha/ovis#2060: Defines ordered work and answer segments, including the distinction between settled and live work heads.

Suggested reviewers: jpcarranza94

Merge Risk: Minimal · up to 0cbeb

No actionable merge-blocking risk is identified in this narrowly scoped QA synchronization change.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title names the specific change: the script now waits for the held Working head to detach before folding the settled turn. It matches the single test-only change in scripts/chat-browser-qa.ts. The…
Description check		The description covers the problem, the root cause, the fix, and validation. It follows the template's Change and Validation sections, and it links the issue (Fixes #2654). It names the checks run and…
Linked Issues check		Issue #2654 is the only directly linked issue, and it is open. Its coding requirement is that scripts/chat-browser-qa.ts must not fail strict mode at await work.click() (line 121). The settled wor…
Out of Scope Changes check		The whole-PR diff against merge base 59e51d0 changes one file, scripts/chat-browser-qa.ts, with three added lines. The added lines are a comment pair and the detach wait for `.chat-activity, .chat-…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…

Full details: Title check

Explanation

The title names the specific change: the script now waits for the held Working head to detach before folding the settled turn. It matches the single test-only change in scripts/chat-browser-qa.ts. The title is 102 characters, which is long, but it stays specific and descriptive.

Full details: Description check

Explanation

The description covers the problem, the root cause, the fix, and validation. It follows the template's Change and Validation sections, and it links the issue (Fixes #2654). It names the checks run and the checks skipped, with reasons. The template's CONTRIBUTING.md and changelog pointers are addressed by the note that no changelog fragment is needed because the change is test-only. The description is detailed and complete.

Full details: Linked Issues check

Explanation

Issue #2654 is the only directly linked issue, and it is open. Its coding requirement is that scripts/chat-browser-qa.ts must not fail strict mode at await work.click() (line 121). The settled work head and the footer's "Working…" head both match .work-block-head. The issue names two acceptable approaches: wait for the activity footer to detach before clicking, or change the chat to drop that head. The PR takes the first approach. At the reviewed head, the diff adds await log.locator(".chat-activity, .chat-activity-reserved").waitFor({ state: "detached" }); after the git status wait and before await work.click();. This covers both the visible .chat-activity footer and the .chat-activity-reserved state. The wait runs after report("idle") and the settled head wait, which matches the issue's reproduction order. The PR description and objectives report that the script passed in 2 of 2 runs. I did not run the script, so that result rests on the author's report. The code change is consistent with the issue's stated fix. No non-coding requirement applies to this issue beyond the CI note, which the PR does not need to change.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against merge base 59e51d0 changes one file, scripts/chat-browser-qa.ts, with three added lines. The added lines are a comment pair and the detach wait for .chat-activity, .chat-activity-reserved. Each line serves the linked objective of avoiding the strict-mode click failure. No product code under src/ or server/ changes. No unrelated test, refactor, or documentation change appears in the diff. The PR description states that no changelog fragment is needed because the change is test-only. That matches the diff.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

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

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: fix/chat-browser-qa-main
• File: .coderabbit.yaml
• Commit: 0cbeb19
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: f5f88b98-75e5-4b69-b9fe-79e603b9dd95

Commits

Reviewing files that changed from the base of the PR and between 59e51d0 and 0cbeb19.

Files selected for processing: 1

• scripts/chat-browser-qa.ts

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

[devswha]

Test-only, reviewed by the merge pass: scripts/chat-browser-qa.ts (not run in CI) clicked .work-block-head while the footer's held Working head (PendingWorkHead, intended behaviour) was still in the page, so Playwright's strict mode saw two heads. It now waits on a page signal for that head (.chat-activity, .chat-activity-reserved) to detach before the click, with no fixed sleep. The lane's omo and Codex Terra reviews approve; bun run check fast passed. Fixes #2654.

CodeRabbit reviewed it.