Conversation
Stop rendering plan_update rows inside the scrollable chat transcript and instead pin the latest plan as a collapsible card above the message area in SessionChatPane, covering both the session modal and the note chat sidebar. - acpTranscript.ts: remove plan_update from VISIBLE_STANDALONE_EVENT_KINDS (leaving session_info_update) and add a latestPlan helper modeled on latestAvailableCommands that returns the newest plan snapshot's entries, normalizes unknown statuses to pending (accepting failed defensively), and returns null when no plan or no entries exist. - PlanCard.svelte: new focused component with a clickable header (caret, overall status icon mirroring tool-call semantics, "Plan" title, muted completed/total progress) and per-entry status icons in the body (spinner for in_progress, CircleCheck for completed, CircleAlert for failed, reserved icon slot for pending). Expansion is initialized from defaultExpanded once so streaming plan updates never override the user's toggle. - SessionChatPane.svelte: derive the latest plan from acpMetadataMessages and mount PlanCard above the scrollable .modal-content, defaulting to expanded except on mobile viewports or compact embeds. Remove the now-dead plan branches: acpEventSummary's plan case, the planEntries helper, the plan_update template markup, and the plan CSS. - acpTranscript.test.ts: flip the plan-row surfacing test to assert plan rows are filtered out, and add latestPlan coverage for newest-plan-wins, no plan rows, empty entries, and unknown-status normalization. Verified with vitest (682 passing) and svelte-check/tsc (0 errors, 0 warnings). Signed-off-by: Matt Toohey <contact@matttoohey.com>
Address code review feedback on cacdec3 (pin the latest ACP plan above the chat transcript): - SessionChatPane.svelte: key the PlanCard on session?.id so the expanded/collapsed toggle resets when the pane is reused for a different session, letting defaultExpanded apply per session instead of a collapse in one session leaking into the next. - acpTranscript.ts: drop the redundant Array.isArray(rawEntries) re-check in latestPlan since arrayProp already returns unknown[] | null; use a plain null check to match latestAvailableCommands. - acpTranscript.ts / acpTranscript.test.ts: remove the unused priority field from PlanEntry, its parsing, and the test assertions, since PlanCard never renders it. One raw input in the tests keeps a priority prop to cover that unparsed props are dropped. Verified with vitest (682 passing) and svelte-check (0 errors). Signed-off-by: Matt Toohey <contact@matttoohey.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1fee405d7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+85
to
+86
| .plan-card { | ||
| flex-shrink: 0; |
There was a problem hiding this comment.
When an ACP plan contains many entries or long wrapped descriptions, this non-shrinking card has no maximum height or internal scrolling, while .session-chat-pane is an overflow-hidden flex column. The expanded plan can therefore consume the pane's entire height and leave the transcript and composer clipped in a zero-height scroll area; cap the card/body height and make the entries independently scrollable.
Useful? React with 👍 / 👎.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Keep pending and running status slots stable, bound expanded plans so they cannot consume the session pane, and wrap unusually long step text. Signed-off-by: Matt Toohey <contact@matttoohey.com>
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
1 participant
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.
Summary