refactor(server): move the live-screen tail checks into server/dialogs/live.ts by devswha · Pull Request #1761 · devswha/ovis

devswha · 2026-10-23T13:00:40Z

Wave P1b of the per-agent module split. The plan is .omo/plans/agent-modules-split.md in the maintainer's workspace, and plan-reviewer approved it. It follows P1a (#1755) and C1 (#1760).

This wave moves the live-screen tail checks out of server/prompt.ts into a new leaf module, server/dialogs/live.ts. These are the per-agent checks that say whether a dialog parsed from the screen is still drawn at its bottom. prompt.ts keeps every per-pane map, the queue drivers and every route.

What changes

Moved verbatim, with their doc comments:
- the omo task and spinner patterns (OMO_TASK_ROW_RE through OMO_EMPTY_BOX_RE);
- CLAUDE_SUBMIT_TAIL_RE, omoFooterIsActive and promptTailIsActive;
- the pi and Command Code footer helpers (PI_MENU_HINT_AT_END_RE, PI_INPUT_HINT_AT_END_RE, CMD_HINT_RE, CMD_FOOTER_WORDS, cmdFooterEnds, PI_MODEL_HINT_AT_END_RE, PI_FOOTER_LINES);
- hintAtEnd, parsePrompt and liveScreen.
New export keywords: parsePrompt, promptTailIsActive and liveScreen. prompt.ts imports these three back.
No re-exports: nothing else imported these names from prompt.ts.
Imports: prompt.ts's imports from ./dialogs/* narrow to what it still uses. live.ts imports the same names from the same dialogs modules, plus paneRead from ../herdr/client.ts.
State: none. Nothing that moves holds module state, and no herdrRpc call moves (liveScreen uses the typed paneRead wrapper).

server/prompt.ts goes from 1,530 to 1,334 lines.

Checks

--color-moved: git diff --color-moved=plain --color-moved-ws=allow-indentation-change origin/main...HEAD shows 382 lines as moved. The other 30 lines are:
- live.ts's eleven import lines;
- prompt.ts's narrowed and new import lines;
- the three declarations whose only change is the added export, each shown as removed and added.
No import cycle: bun <plan>/no-path.ts server/dialogs/live.ts server/prompt.ts server/conversation.ts prints "no path to server/prompt.ts, server/conversation.ts". The new module never reaches either file through value imports.
Typecheck: bun run typecheck passes.
Unit tests: 589 pass, 0 fail across 20 files, from:

HERDR_TEST_MODE=unit bun test server/prompt.test.ts server/dialogs/ server/hermes-prompt.test.ts server/cmd.test.ts server/core/

Every parseInteractivePrompt path runs through parsePrompt.
bun run check fast: passes (unit 6253 pass, 0 fail, 13 skip in 443 files, 8 at a time, 73.5s)
Integration lane: CI runs pending-input, prompt, opencode-prompt and claude-session contracts.

Outside PRs that touch the same files

No other maintainer PR touches these files. Three outside contributors' PRs touch server/prompt.ts. Each has a small change inside a function that now lives in server/dialogs/live.ts, so its rebase carries that change into the new file. I checked each with git merge-tree:

fix(chat): recognize Pi's thinking selector #1670 merges cleanly with main, and conflicts with this branch in server/prompt.ts.
fix(prompt): support Pi rpiv question cards and safe navigation #1714 already conflicts with main in server/prompt.ts, and needs a rebase either way.
fix(prompt): support GJC custom answers with guarded delivery #1669 already conflicts with main in server/prompt.ts, and needs a rebase either way.

Move-only: moved declarations plus imports and three export keywords. No changelog fragment: users see no change.

…s/live.ts Move the OmO task-row regexes, omoFooterIsActive, promptTailIsActive, the pi and Command Code footer helpers, hintAtEnd, parsePrompt and liveScreen verbatim. live.ts exports parsePrompt, promptTailIsActive and liveScreen; prompt.ts imports them. prompt.ts keeps every map and route; no re-exports were needed. Plan: .omo/plans/agent-modules-split.md P1b.

coderabbitai · 2026-10-23T13:01:12Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Source mode: Auto
Repository: devswha/ovis
Branch: refactor/dialogs-live-module
File: .coderabbit.yaml
Commit: 0f4ec6b
Layers: Repository YAML
Review profile: CHILL
Plan: Advanced
Run ID: 9c52a6e2-7a65-4c4f-8442-4417e710311a

📥 Commits

Reviewing files that changed from the base of the PR and between a0e32ae and 0f4ec6b.

📒 Files selected for processing: 2

server/dialogs/live.ts
server/prompt.ts

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

📝 Summary

Summary by CodeRabbit

Bug Fixes
- Improved detection of active prompts at the end of a live screen, including wrapped prompts and agent-specific footers.
- Improved handling of prompts shown in task widgets and permission dialogs, helping keep the displayed prompt state aligned with what is currently visible.

The PR moves live prompt detection and screen reading from server/prompt.ts to server/dialogs/live.ts. The new module checks responder-specific prompt tails, parses registered non-result readers, and reads the pane’s detection buffer as text.

Changes: Live prompt detection

Layer / File(s)	Summary
Detect active prompt tails `server/dialogs/live.ts`, `server/prompt.ts`	`promptTailIsActive` checks responder-specific prompts and permitted footer content. `server/prompt.ts` removes its local tail-detection implementation and related patterns.
Parse prompts and read the live screen `server/dialogs/live.ts`, `server/prompt.ts`	`parsePrompt` returns the first candidate whose prompt tail is active, or `null`. `liveScreen` reads the pane’s detection buffer as text. `server/prompt.ts` imports these helpers and removes its local `liveScreen` implementation.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Possibly related issues

devswha/ovis#1644: The issue identifies promptTailIsActive as the cause of missing OMP Ask cards. This PR relocates the helper but does not state that it changes OMP Ask parsing behavior.

Suggested reviewers: diegolhambi

Merge Risk: ⚪ Minimal · up to 0f4ec

No merge-blocking behavior change is established; the extraction appears ready for normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

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

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main refactor: moving live-screen tail checks into server/dialogs/live.ts.
Description check		The description explains the change, resulting behavior, affected modules, validation results, integration coverage, and merge considerations. The headings differ from the template, but the required i…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the change, resulting behavior, affected modules, validation results, integration coverage, and merge considerations. The headings differ from the template, but the required information is present.

✨ Finishing Touches: 💡 1

📝 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

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

devswha · 2026-10-23T13:08:41Z

Merge pass: move-only PR (per-agent split, wave P1b), merged on its checks without waiting for a CodeRabbit review, as AGENTS.md allows.

Pure move. A line-multiset comparison differs only in imports and the three export keywords (parsePrompt, promptTailIsActive, liveScreen).
Gate review by a reviewer on a different model than the author's found:
- no test mock or spy is bypassed;
- no runtime import cycle, and registry initialization is unchanged;
- no module state moved;
- no behaviour change.
Tests. The merge pass ran the suite at this head: 589 pass across 20 files (prompt, dialogs, hermes-prompt, cmd, core). The typecheck passes, and all 47 conventions tests pass.
Nit, not blocking. The doc comment above CLAUDE_SUBMIT_TAIL_RE is duplicated at server/dialogs/live.ts:28-29. Drop one line in the next wave.
Overlap. Outside contributors fix(prompt): support GJC custom answers with guarded delivery #1669, fix(chat): recognize Pi's thinking selector #1670 and fix(prompt): support Pi rpiv question cards and safe navigation #1714 touch server/prompt.ts. Their changes now go to server/dialogs/live.ts on rebase.

All required checks are green and no review thread is open.

…ersation-shared.ts (#1763) Wave C2 of the per-agent module split. The plan is `.omo/plans/agent-modules-split.md` in the maintainer's workspace, and plan-reviewer approved it. It follows C1 (#1760) and P1b (#1761). This wave moves the conversation types and constants that agent modules share out of `server/conversation.ts` into a new leaf module, `server/conversation-shared.ts`. Later waves (Devin's session lookup, then the Claude and omp sessions) can then throw `ConversationUnavailable` without importing `conversation.ts`. ## What changes - **Moved verbatim, with their doc comments:** - `TRANSCRIPT_WINDOW_BYTES`; - the error classes `ConversationUnavailable`, `ConversationNotStarted` and `HistoryChanged`; - the types `RecognizedConversation`, `StreamSource` and `ConversationPage`; - `PROCESS_VERSION` and `answerVersion`. - **State:** `PROCESS_VERSION` holds one `randomUUID()`, set once and never changed. It moves with its only user, `answerVersion`, so it still has a single owner. - **New `export`:** only `StreamSource` gains one, because `conversation.ts` imports it back. - **Re-exports:** `conversation.ts` re-exports every name that was exported before, so no other importer changes. - **One import path changes on purpose:** `server/hermes.ts` now imports the same names from `./conversation-shared.ts` instead of `./conversation.ts`. That removes the runtime import cycle between `hermes.ts` and `conversation.ts`. ## Checks - **`--color-moved`:** `git diff --color-moved=plain --color-moved-ws=allow-indentation-change origin/main...HEAD` shows 138 lines as moved. The other 8 lines are: - the new file's two import lines; - `StreamSource` with its added `export`, shown as removed and added; - `conversation.ts`'s import and re-export lines; - `hermes.ts`'s changed module path. - **No cycles:** checked with `bun <plan>/no-path.ts`. - With `server/conversation-shared.ts server/conversation.ts server/prompt.ts`, it prints "no path to server/conversation.ts, server/prompt.ts". - With `server/hermes.ts server/conversation.ts`, it prints "no path to server/conversation.ts". The cycle is gone. - **Typecheck:** `bun run typecheck` passes. - **Unit tests:** `HERDR_TEST_MODE=unit bun test server/hermes.test.ts server/conversation.test.ts server/chat-history.test.ts`: 177 pass, 0 fail. - `bun run check fast`: passes (unit 6253 pass, 0 fail, 13 skip in 443 files, 8 at a time, 66.5s) - **Integration lane:** CI runs the gjc, api, opencode and hermes contracts. ## Outside PRs that touch the same files No other maintainer PR touches these files. Three outside contributors' PRs touch `server/conversation.ts`. Each adds a member to `RecognizedConversation`'s `source` union, which now lives in `server/conversation-shared.ts`, so a rebase carries that one line into the new file. I checked each with `git merge-tree`: - **#1706** merges cleanly with main, and conflicts with this branch in `server/conversation.ts`. - **#1652** already conflicts with main in `server/conversation.ts`, and needs a rebase either way. - **#1442** already conflicts with main in `server/conversation.ts`, and needs a rebase either way. Move-only: moved declarations plus imports, one re-export line, one added `export`, and the `hermes.ts` module path. No changelog fragment: users see no change.

devswha merged commit d736944 into main Oct 23, 2026
17 checks passed

devswha deleted the refactor/dialogs-live-module branch October 23, 2026 13:08

devswha mentioned this pull request Oct 23, 2026

refactor(server): move the shared conversation types into server/conversation-shared.ts #1763

Merged

Navigation Menu

Repository navigation

refactor(server): move the live-screen tail checks into server/dialogs/live.ts - #1761

refactor(server): move the live-screen tail checks into server/dialogs/live.ts#1761
devswha merged 1 commit into
mainfrom
refactor/dialogs-live-module

devswha commented Oct 23, 2026

Uh oh!

coderabbitai Bot commented Oct 23, 2026 •

edited

Loading

Summary by CodeRabbit

Uh oh!

devswha commented Oct 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

devswha commented Oct 23, 2026

What changes

Checks

Outside PRs that touch the same files

Uh oh!

coderabbitai Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

devswha commented Oct 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Oct 23, 2026 •

edited

Loading