Skip to content

Render file change rows in thread conversation#9

Open
SHAREN wants to merge 10 commits intofriuns2:mainfrom
SHAREN:codex/bug-010-file-change-rows
Open

Render file change rows in thread conversation#9
SHAREN wants to merge 10 commits intofriuns2:mainfrom
SHAREN:codex/bug-010-file-change-rows

Conversation

@SHAREN
Copy link
Contributor

@SHAREN SHAREN commented Mar 23, 2026

Summary

  • render persisted fileChange items from thread/read as inline expandable rows in ThreadConversation
  • preserve live fileChange updates in the shared live system-message pipeline so active edits show up inside the thread during a running turn
  • keep the PR scoped to the original BUG-010 fix stack with no codex-web-local history mixed in

Testing

  • npm run build
  • node output/playwright/verify-file-change.mjs
  • node output/playwright/verify-file-change-live.mjs

@qodo-code-review
Copy link

Review Summary by Qodo

Render file change rows in thread conversation

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Render persisted file change items from thread/read as inline expandable rows
• Parse and normalize file change data with diff statistics extraction
• Preserve live file change updates in shared system-message pipeline
• Display file change summaries with verb, path, and line statistics
Diagram
flowchart LR
  ThreadItem["ThreadItem fileChange"] -->|normalizeThreadItemV2| UiMessage["UiMessage with fileChange"]
  UiMessage -->|live updates| LiveCommand["Live command pipeline"]
  LiveCommand -->|upsertLiveCommand| ThreadConversation["ThreadConversation component"]
  ThreadConversation -->|render| FileChangeRow["Expandable file change row"]
  FileChangeRow -->|display| DiffView["Diff viewer with stats"]
Loading

Grey Divider

File Changes

1. src/types/codex.ts ✨ Enhancement +12/-0

Add file change data types to UiMessage

• Added UiFileChangeData type with file change metadata and diff statistics
• Extended UiMessage type with optional fileChange field
• Includes path, kind, status, diff, movePath, linesAdded, linesRemoved, openLine

src/types/codex.ts


2. src/api/normalizers/v2.ts ✨ Enhancement +132/-1

Normalize file change items with diff statistics

• Added import of UiFileChangeData type
• Implemented toUiMessages handler for fileChange items with per-change message generation
• Added normalizeFileChangeStatus to validate file change status values
• Added readFileChangeKind to parse file change kind and move_path
• Added readFileChangeDiffStats to extract line counts and opening line from unified diff format
• Exported new normalizeThreadItemV2 function for live item normalization

src/api/normalizers/v2.ts


3. src/composables/useDesktopState.ts ✨ Enhancement +54/-0

Handle live file change notifications and equality checks

• Added imports for ThreadItem and normalizeThreadItemV2
• Added UiFileChangeData type import
• Implemented areFileAttachmentsEqual comparison function
• Implemented areFileChangeDataEqual comparison function for live updates
• Updated areMessageFieldsEqual to compare fileAttachments and fileChange fields
• Added readFileChangeMessages to extract file change notifications from RPC events
• Added upsertLiveCommand calls for file change messages in notification handler

src/composables/useDesktopState.ts


View more (1)
4. src/components/content/ThreadConversation.vue ✨ Enhancement +183/-1

Render expandable file change rows in conversation

• Added file change message rendering with expandable row UI
• Implemented file change expansion state management with expandedFileChangeIds
• Added helper functions for file change display: verb, path, stats, diff text
• Added styling for file change rows with chevron, verb, path, and line statistics
• Added file change metadata display with browse link and move path info
• Reset file change expansion state when switching threads

src/components/content/ThreadConversation.vue


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

qodo-code-review bot commented Mar 23, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. FileChange status mis-normalized🐞 Bug ✓ Correctness
Description
normalizeFileChangeStatus silently maps any unrecognized status (including potential snake_case
'in_progress') to 'completed', causing the UI to show the wrong verb/state for file changes. This is
inconsistent with normalizeCommandStatus, which explicitly normalizes 'in_progress' to 'inProgress'.
Code

src/api/normalizers/v2.ts[R209-214]

+function normalizeFileChangeStatus(value: unknown): UiFileChangeData['status'] {
+  if (value === 'inProgress' || value === 'completed' || value === 'failed' || value === 'declined') {
+    return value
+  }
+  return 'completed'
+}
Evidence
In src/api/normalizers/v2.ts, normalizeCommandStatus already handles both 'inProgress' and
'in_progress', proving the client expects snake_case status values in some responses. The new
normalizeFileChangeStatus lacks that mapping and defaults unknown values to 'completed', which will
mislabel any fileChange item that arrives with 'in_progress'. The schema for fileChange status is
PatchApplyStatus (inProgress/completed/failed/declined), so the intended in-progress state is
well-defined and should be preserved when encountered in snake_case.

src/api/normalizers/v2.ts[203-206]
src/api/normalizers/v2.ts[209-214]
documentation/app-server-schemas/typescript/v2/PatchApplyStatus.ts[1-5]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`normalizeFileChangeStatus` currently accepts only camelCase status values and defaults everything else to `'completed'`. Elsewhere (`normalizeCommandStatus`) the client normalizes `'in_progress'` -> `'inProgress'`, so fileChange should be consistent to avoid mis-rendering live/persisted fileChange items if snake_case appears.
### Issue Context
- `fileChange.status` comes from app-server `PatchApplyStatus`.
- Client already anticipates snake_case for status in `normalizeCommandStatus`.
### Fix Focus Areas
- src/api/normalizers/v2.ts[203-214]
### Suggested change
- Update `normalizeFileChangeStatus` to accept `'in_progress'` and map it to `'inProgress'` (mirroring `normalizeCommandStatus`).
- Consider logging/telemetry on unknown statuses rather than silently coercing to `'completed'`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@friuns2
Copy link
Owner

friuns2 commented Mar 23, 2026

IMG_20260323_191208_524
На мобиле поплыло) гляну когда дома буду

@SHAREN
Copy link
Contributor Author

SHAREN commented Mar 23, 2026

@friuns2 короче я "навайбкодил" у себя в локальном проекте, но там маленько порядок у меня нарушился, но короче говоря еще несколько фич добавил. fast speed mode, drag-n-drop, вставку изображекний из буфера обмена, отображения хода сжатия контекста, частичную поддержку с codex app. Ну и сейчас понял короче что теперь тяжело мне будет разбираться чтобы сделать правильно PR чтобы ничего не забыть как сейчас 🤯

я постараюсь аккуратно теперь с этим разобраться

@friuns2
Copy link
Owner

friuns2 commented Mar 23, 2026

ну мне мерджить или подождать?

@SHAREN
Copy link
Contributor Author

SHAREN commented Mar 23, 2026

ну мне мерджить или подождать?

подождать, но я позже разберусь

@SHAREN
Copy link
Contributor Author

SHAREN commented Mar 24, 2026

Дозалил недостающие правки из локального репозитория и перепроверил ветку заново.

Что обновил:

  • basename-only summary path для file-change rows
  • inline layout без mobile overflow
  • hidden collapsed details без bleed
  • dark-theme стили для file-change rows
  • live file-change rows теперь не дублируют overlay и не пропадают до синка persisted history

Проверка:

  • npm run build
  • node output/playwright/verify-file-change.mjs
  • node output/playwright/verify-file-change-live.mjs
  • node output/playwright/verify-file-change-snake-case.mjs
  • node output/playwright/verify-file-change-theme-matrix.mjs

Отдельно прогнал light/dark + desktop/mobile/tablet.

@friuns2
Copy link
Owner

friuns2 commented Mar 24, 2026

image ты добавил новый баг, старые команды не изчезают как раньше теперь

@friuns2
Copy link
Owner

friuns2 commented Mar 24, 2026

и rebase надо сделать там конфликты появились

SHAREN added 10 commits March 25, 2026 18:59
What changed:
- restored basename-first inline row layout and browse-line linking for file-change summaries
- hid collapsed detail bleed and added dark-theme overrides for the file-change renderer
- kept live file-change rows merged into the thread and suppressed the duplicate live overlay while inline rows are present

Why:
- the clean PR branch missed part of the original local fixes, which left the upstream PR with mobile/layout regressions and incomplete live behavior

Impact:
- file-change rows now stay compact on small screens, read correctly in dark mode, and expand without detail bleed
- live file-change rows remain visible through turn completion until persisted history catches up, avoiding duplicate or flickering UI

Validation:
- npm run build
- node output/playwright/verify-file-change.mjs
- node output/playwright/verify-file-change-live.mjs
- node output/playwright/verify-file-change-snake-case.mjs
- node output/playwright/verify-file-change-theme-matrix.mjs
What changed:
- added a shared command resolution helper that can find the installed Codex CLI from Windows npm locations, including the vendored codex.exe path
- updated the CLI bootstrap to reuse that resolver and to repair PATH handling for user-prefix npm installs on Windows
- updated the app-server bridge and schema generation path to stop hardcoding spawn('codex', ...) and use the resolved executable instead

Why:
- the clean clone could build, but it could not launch the real Codex app-server on this machine because Node spawn could not resolve the Windows shell shim reliably
- that left http://127.0.0.1:4175 serving the shell without real threads

Impact:
- the clean clone can now serve real Codex data on both localhost and the LAN bind address
- this stays local to the runtime branch and does not alter the upstream PR branch

Validation:
- npm run build
- verified http://127.0.0.1:4175/ and http://192.168.0.119:4175/ return 200
- verified /codex-api/rpc thread/list returns real threads
- captured headless Playwright screenshots for local light, local dark, LAN light, and an opened real thread in light/dark
(cherry picked from commit 6431531c3488b14aaf172d2631985e3742a1f274)
What changed:
- tracked the live arrival order of assistant, command, and file-change items per thread
- merged current-turn persisted messages against that live order instead of falling back to raw persisted ordering after the stream ends
- kept persisted-only items in place while reordering known live items and inserting live-only items near their ordered neighbors

Why:
- PR9 no longer hard-jumped the scroll position, but completed turns could still reorder into assistant text first with cmd/file rows sinking to the bottom of the turn

Impact:
- Edited rows and command rows stay in their live sequence even after the turn finishes and live buffers are cleared
- the thread no longer regresses into a trailing cmd/file tail once persisted messages replace the live state

Validation:
- npm run build
- live Playwright check on http://localhost:4176/#/thread/019d24ab-cd48-7890-bcad-21dbd2ca9414 with a single open page and a 3-cycle edit/whoami/report prompt; sawCmdWhileOpen=true, sawFileWhileOpen=true, trailingKinds=[]

(cherry picked from commit 68f0b6b28c1811da496cd6124280832dcda9f130)
What changed:
- derived and stored the full displayed order of the current turn instead of treating the order cache as a live-only item queue
- refreshed that cached turn order whenever persisted messages, live agent messages, or live command/file rows changed
- reused the cached displayed order when merging persisted assistant history with live-only command and file-change rows

Why:
- thread/read persists assistant messages for these turns but does not persist the commandExecution and fileChange rows
- when the persisted assistant history arrived after the stream, it could push already-visible Edited/cmd rows down into a bottom tail

Impact:
- Edited and command rows keep the on-screen order they reached during the live turn
- persisted assistant updates no longer collapse those rows to the bottom after Worked and the final assistant message

Validation:
- npm run build
- live Playwright check on http://localhost:4176/#/thread/019d24ab-cd48-7890-bcad-21dbd2ca9414 with the 3-cycle edit/whoami/report prompt; sawCmdWhileOpen=true, sawFileWhileOpen=true, trailingKinds=[]
- artifact: D:\RENAT\Documents\codexui-fork\output\playwright\pr9-triplet-live-order-summary-1774446177592.json

(cherry picked from commit 40e036c71080331a268b1397d874507ba02ac8a6)
What changed:
- reintroduced the small npm PATH helper functions used by the CLI install fallback
- kept the rebased branch on the newer utils/commandInvocation path instead of reviving the removed commandResolution module

Why:
- the PR branch started failing to build after rebasing onto upstream/main because the install fallback still referenced helper functions that were no longer imported

Impact:
- the rebased PR branch builds cleanly again
- Windows-friendly PATH updates stay intact for the CLI install fallback

Validation:
- npm run build
Add the screenshot captured from the live PR9 verification flow so the GitHub comment can embed visual proof of the final Edited/cmd ordering fix.
@SHAREN SHAREN force-pushed the codex/bug-010-file-change-rows branch from 4890668 to 7646637 Compare March 25, 2026 14:08
@SHAREN
Copy link
Contributor Author

SHAREN commented Mar 25, 2026

Дозалил фиксы по багу из комментария про то, что старые Edited / cmd ряды зависают и скапливаются хвостом внизу, и заодно сделал ребейз ветки на актуальный main.

Что обновил:

  • rebased codex/bug-010-file-change-rows на текущий upstream/main и разрулил конфликты
  • добавил фиксы для PR9, которые удерживают scroll-позицию и live-порядок внутри активного turn
  • отдельно зафиксировал display-order текущего turn-а, чтобы persisted assistant history больше не выталкивал уже показанные Edited/cmd вниз хвостом
  • после ребейза восстановил маленький CLI PATH helper fix, чтобы ветка снова собиралась

Что это чинит по сути:

  • thread/read сохраняет assistant history, но не сохраняет commandExecution / fileChange items
  • из-за этого, когда persisted assistant messages приезжали после live-стрима, уже видимые Edited и команды могли съехать вниз под Worked и финальный assistant text
  • теперь UI держит полный display-order текущего turn-а и использует его при merge persisted/live состояния, поэтому rows остаются в том порядке, в котором дошли на экране

Проверка:

  • npm run build
  • live-проверка на http://localhost:4176/#/thread/019d24ab-cd48-7890-bcad-21dbd2ca9414
  • сценарий: 3 цикла edit -> whoami -> short sentence, на открытом треде, с ожиданием стабилизации DOM, а не по кнопке stop
  • результат: sawCmdWhileOpen=true, sawFileWhileOpen=true, trailingKinds=[]

Скрин после фикса:

PR9 live order fix

@friuns2
Copy link
Owner

friuns2 commented Mar 25, 2026

image

когда экспанд делаешь worked а там дубликат, тшателнее тестируй пожалуйста а то потом сложнее будет фиксить если пр пройдет

@friuns2
Copy link
Owner

friuns2 commented Mar 25, 2026

на модели codex 5.2 тестируй тоже

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants