Skip to content

fix(editor): improve rich markdown table readability - #827

Merged
AmethystLiang merged 3 commits into
stablyai:mainfrom
aparusel:fix/rich-markdown-table-borders
Apr 19, 2026
Merged

AmethystLiang merged 3 commits into
stablyai:mainfrom
aparusel:fix/rich-markdown-table-borders

Conversation

@aparusel

Copy link
Copy Markdown
Contributor

Summary

  • add visible cell borders to rich markdown tables so formatted tables are easier to scan in the editor
  • add subtle header and alternating row styling to improve separation without changing markdown content or table behavior
  • keep the change scoped to the rich markdown surface; preview mode already had table separators

Why

Rich markdown tables were hard to read because adjacent cells visually ran together. Adding clear cell boundaries greatly enhances readability, especially for wider tables with dense content.

Testing

  • corepack pnpm lint
  • corepack pnpm typecheck
  • corepack pnpm test
  • corepack pnpm build:relay && corepack pnpm build:electron-vite && corepack pnpm build:cli

Screenshots

  • Omitted for now; can be added manually after PR creation if needed.

AI Code Review Summary

  • Reviewed the change for cross-platform compatibility: CSS-only styling scoped to .rich-markdown-editor with no macOS/Linux/Windows-specific code paths, shortcuts, or path handling.
  • Checked for regressions in adjacent markdown surfaces: preview mode already had table borders, and this update only affects the rich editor table presentation.
  • Basic security audit: no new network access, file system access, HTML injection, or privilege boundaries changed; the patch only adjusts static table styling.

Platform Notes

  • Expected to behave the same on macOS, Linux, and Windows because the change only updates shared renderer CSS.
  • Verified locally in Orca dev mode on macOS.

X Handle

  • @a_parusel

Show borders and subtle striping for rich markdown tables so cell boundaries stay legible in the formatted editor.
@aparusel

Copy link
Copy Markdown
Contributor Author

Screenshot before my change:
orca-md-without-table-styles

Screenshot after my change:
orca-md-with-table-styles

IMO readability is a lot better after the change.

Keep the new table styling readable without forcing header text to uppercase.
@aparusel

Copy link
Copy Markdown
Contributor Author

Edit: added another commit removing the uppercase titles to make the PR less invasive.

@aparusel

Copy link
Copy Markdown
Contributor Author

Looks great in Dark-Mode too:

Bildschirmfoto 2026-04-19 um 14 04 08

@AmethystLiang
AmethystLiang self-requested a review April 19, 2026 16:04
Switch zebra striping to nth-child(odd) so the shaded row doesn't sit
directly under the header, and drop the compounding th font-size.

@AmethystLiang AmethystLiang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow huge quality of life improvement! thx for the PR.

Will get this out in today's release

@AmethystLiang
AmethystLiang merged commit b8b773e into stablyai:main Apr 19, 2026
nwparker added a commit that referenced this pull request Sep 2, 2026
…re-probe

Two gaps in the fd-retirement change.

`CustomWriteStream` is the third fd-addressed surface and takes its own plain-number
copy of the master at spawn, so `Terminal._fd = -1` never reaches it. Its `dispose()`
only cleared a pending immediate, and it was called from `destroy()` alone — the
EIO/EOF read-error path and the socket `'close'` path both reach `_close()` without
it. A residual `_writeQueue` and an in-flight `fs.write` therefore kept addressing
the number after libuv gave it up. Retire the copy and drop the queue in `dispose()`,
and call it from `_close()`.

Restore the relay's catch-block re-probe. It was never redundant with the patch:
libuv closes the fd synchronously inside `uv_close`, before the JS `'close'` that
runs `_close()`, so the reuse window opens ahead of any JS guard; and a relay host
installs node-pty from npm, where the patch is not applied at all. The re-probe is a
`process.kill(pid, 0)` on a path already inside an exception handler.

Both existing resize tests pinned liveness with a constant mock, which is why the
deletion passed CI invisibly; the new case is alive at the pre-probe and gone by the
time the ioctl throws.

Also adds the patch's first upstream reference: microsoft/node-pty#220 named this
mechanism in 2018 and was closed 2025-12-19 as completed after only improving the
error message; #827 is still open. Tracked as #18109.
nwparker added a commit that referenced this pull request Sep 2, 2026
…#17930)

* fix(native-chat): keep the attachments on a Claude turn that pasted images (#17801)

* fix(native-chat): keep the attachments on a Claude turn that pasted images

A Claude turn carrying pasted images reached native chat with no images at all —
no thumbnails on mobile, and not even an attachment chip on desktop. Nothing
showed that the message had any.

Both carriers were being dropped:

- Claude records the paths in a companion turn marked `isMeta`, holding one
  `[Image: source: <path>]` text block per image. The decoder treats an `isMeta`
  user row as injected, filters it down to tool-result blocks, and returns null
  when none remain — so the whole row went away.
- The prompt row's own `image` blocks are `{source: {type: 'base64'}}`, which
  carry no url or path, so `imageRefBlock` drops them too.

With the companion gone, `isImageSourceUserTurn` could never fire and the fold in
`normalizeImageTranscriptMessages` was unreachable on the Claude path.

Surveying every transcript under `~/.claude/projects`: 238 of 241 image-source
rows are `isMeta`, across every versioned release (2.1.220 through 2.1.237); the
3 that are not carry no version field at all. 38 of those rows hold more than one
content block, which also defeated the single-block rule in
`isImageSourceUserTurn`.

Let image-source text survive the injected-turn filter, and recognize a turn
whose blocks are *all* markers rather than only a lone one. An ordinary injected
turn (a skill preamble, a compact summary) is still dropped, and a turn that
mixes prose with a marker is still not an image-source turn.

Carrying the paths keeps the payload small; decoding the base64 instead would put
hundreds of KB per image on the wire to mobile.

* fix(native-chat): preserve image companion ordering

* fix(native-chat): keep image companions turn-local

---------

Co-authored-by: Merge Sim <sim@local>

* fix(native-chat): stop unjournaled provider frames from killing the session (#17813)

A frame the classifier declines (status-chrome, suppressed-benign, stream-into-item)
is deliberately not journaled. #17720 turned that null translation into
`{accepted: false, reason: 'untranslated'}`, which is not `backpressure`, so the
notification retry queue treated it as unreplayable and escalated through fail() ->
forceCloseUnexpected -> connection.close(). The app-server latched `closing` and the
create path's next model/list rejected with "codex app-server connection is closed
(model/list)".

The provider emits `remoteControl/status/changed` right after initialize, so every
structured Codex session died on its first chrome frame. Admit the null translation
instead, before any bookkeeping or publish.

Also restore the error-frame exemption from the generic row cap, dropped by the same
PR: the cap now runs after the classification check, so a noisy turn can no longer
reduce provider errors to a suppression count. The test that pinned the capped
behavior is inverted to assert the exemption.

Co-authored-by: Merge Sim <sim@local>

* refactor(mobile): split session and terminal surfaces

(cherry picked from commit f129f29)

* fix(mobile): restore session parity after extraction

(cherry picked from commit 47706a5)

* chore(mobile): prune stale max-lines overrides

(cherry picked from commit be954c3663409116f34ba08658876262733b3f67)

* fix(mobile): honor host-follow tab snapshots

(cherry picked from commit a4a9c4da19d6967eceb8025dd01480a56261039f)

* fix(mobile): keep split session hooks render-pure

Documents the pre-existing render-time refs the split relocated onto changed
lines, and drops a ref assignment the split added that the monolith never had.

* fix(agent-send-target): acknowledge delivery even when picker closes bef (#17835)

Delivery callbacks and telemetry belong to the completed send operation, not to the
picker instance that launched it. Remove early returns that skipped delivery
acknowledgment and success toast when the popover was already closed.

* fix(mobile-native-chat): retire an image echo glued with the send beside it (#17783)

* fix(mobile-native-chat): retire an image echo glued with the send beside it

A message sent with images could render two or three times over, with the copy
carrying the photos sorting below the reply that answered it — and it never
cleared.

A send issued while the agent is mid-turn is glued onto the agent's input line
with any send adjacent to it, so the pair lands as one transcript row whose text
is the concatenation. Every retirement path then declined the pair:

- The image matcher wanted the whole row to equal the echo's text, so a glued
  row never bound. That also stranded the local preview: the phone's photo never
  reached the authoritative row.
- The exact-count path skips image echoes by design.
- The glue path excluded image echoes too, which made one a *barrier* — splitting
  the run so the text-only send beside it was left alone, and a lone match is
  rejected as an ordinary landing.

So neither echo could ever retire, and the unmatched image echo fell through to
the trailing bucket, which is what put it below the reply.

Match a glued row in the image matcher, and let an image echo take part in the
glue pass once its preview has been rebound. It stays a barrier while unbound,
so the existing guarantee is kept: an image echo never retires before its local
preview reaches the transcript row, or the photo would disappear.

* fix(mobile-chat): require image provenance for glued prefix matches

---------

Co-authored-by: Merge Sim <sim@local>

* refactor(renderer): split oversized UI surfaces

(cherry picked from commit da89be4)

* fix(renderer): keep split imports lint-clean

(cherry picked from commit c778ac7)

* fix(renderer): preserve extracted lifecycle and retention behavior

(cherry picked from commit 1731f2a)

* chore(max-lines): prune refactored file suppressions

(cherry picked from commit 787228e)

* fix(status-bar): preserve Git refresh ordering after split

(cherry picked from commit e65d298)

* fix(lint): preserve TaskPage effect suppressions after split

(cherry picked from commit 4a3bc23)

* fix(status-bar): preserve workspace-space review and agent freshness

(cherry picked from commit 9455e31)

* fix(automations): preserve Escape drill-out behavior

(cherry picked from commit 790680d)

* fix(renderer): restore worktree palette behavior after split

(cherry picked from commit 0b78c1c)

* style(renderer): format palette project candidates

(cherry picked from commit 1e93d66)

* refactor(renderer): finish worktree palette extraction

(cherry picked from commit ea8e6e5)

* fix(renderer): document intentional render-time refs

(cherry picked from commit 6fc78dd)

* chore(max-lines): prune stale lifecycle baseline

(cherry picked from commit 42bf3eb)

* fix(renderer): preserve status localization parity

(cherry picked from commit 0a802c7)

* fix(renderer): preserve floating editor visibility

(cherry picked from commit bcf715b)

* fix(automations): preserve destination form save semantics

(cherry picked from commit a793b07)

* fix(renderer): preserve Codex sign-in behavior

(cherry picked from commit 81f4803)

* fix(renderer): preserve palette create telemetry

(cherry picked from commit b13c2b1)

* style(renderer): format extracted palette modules

* refactor(automations): extract list toolbar and drop dead list projection

* fix(lint): drop unused exhaustive-deps suppression after split

The split's dependency list no longer trips the rule, so the directive is dead
and the changed-code quality gate rejects it.

* fix(renderer): document pre-split render-time refs in workspace search

Both assignments are verbatim from the pre-split monolith; the split relocated
them onto changed lines, which the React Doctor gate scans.

* refactor(renderer): split runtime and store modules

(cherry picked from commit 207bef0)

* fix(renderer): merge split runtime imports

(cherry picked from commit 90bf505)

* chore(max-lines): prune runtime baseline entries

(cherry picked from commit 08820a4)

* fix(renderer): preserve accepted visibility snapshots

(cherry picked from commit bc5f8a5ac5b8e8ad3fc59250795842e663bad0ee)

* refactor(preload): split bridge API modules

(cherry picked from commit b77e318)

* test(preload): census split GitHub bridge owners

(cherry picked from commit cd08e4e)

* fix(preload): merge split plugin imports

(cherry picked from commit 682ff4f)

* chore(max-lines): prune preload baseline entry

(cherry picked from commit aad2e1e)

* fix(pty,remote): close the pty master fd leak and two remote-terminal defects

#8362: node-pty leaves the forkpty()/posix_openpt() master without FD_CLOEXEC,
so on Linux every later child of the process -- both later pty children and
plain child_process spawns -- inherits it and keeps the /dev/pts device alive.
Measured on Linux with stock node-pty 1.1.0: master fd flags 0404002
(cloexec=false), and 17 -> /dev/pts/ptmx present in both a later pty child's
/proc/self/fd and a later child_process child's. Extend the existing node-pty
patch with pty_cloexec() on both PtyFork spawn paths; after the patch the flags
read 02404002 (cloexec=true) and neither child sees the master. This covers the
app and terminal daemon only -- the SSH relay installs node-pty from npm on the
remote host, so it stays exposed (see the report).

#8414: the inspection queue's fire-and-forget `.finally()` chain re-raised a
rejecting inspection as a renderer-global unhandledrejection, which an
unreachable runtime produced on every cadence tick.

#9194: `tab_not_found` is the host's definitive "no such tab", but the close
path cleared the close intent for it exactly like a dropped connection, so a
host that keeps republishing the dead surface re-materialized the pane the user
just closed. Keep that intent and drop its TTL. Also route the banner's
"Remote terminal was closed." line through translate() so it stops mixing
English into a localized banner.

* fix(pty): retire the node-pty master fd instead of probing around it

node-pty hands the master to libuv, which closes it on EIO/EOF, but `_close()`
never invalidated `_fd` and neither `resize()` nor the `process` getter consulted
anything. A late ioctl therefore addressed whatever the kernel had since reissued
that number to. On Linux the very next pty gets it: resizing an exited handle
silently resized an unrelated live pane, with no error for #17832's
probe-retry-classify to observe.

Retire `_fd` in the same block that gives up the master and guard both fd-addressed
syscalls on it, so a stale handle is unreachable by construction. The relay's resize
keeps the isProcessAlive-proven retirement — still the only evidence of `exited` —
and drops the re-probe-and-classify arm the ioctl no longer needs.

Windows is unaffected: WindowsTerminal.resize goes through the conpty agent and
reads no fd.

* fix(pty): retire the write stream's fd copy too, and keep the resize re-probe

Two gaps in the fd-retirement change.

`CustomWriteStream` is the third fd-addressed surface and takes its own plain-number
copy of the master at spawn, so `Terminal._fd = -1` never reaches it. Its `dispose()`
only cleared a pending immediate, and it was called from `destroy()` alone — the
EIO/EOF read-error path and the socket `'close'` path both reach `_close()` without
it. A residual `_writeQueue` and an in-flight `fs.write` therefore kept addressing
the number after libuv gave it up. Retire the copy and drop the queue in `dispose()`,
and call it from `_close()`.

Restore the relay's catch-block re-probe. It was never redundant with the patch:
libuv closes the fd synchronously inside `uv_close`, before the JS `'close'` that
runs `_close()`, so the reuse window opens ahead of any JS guard; and a relay host
installs node-pty from npm, where the patch is not applied at all. The re-probe is a
`process.kill(pid, 0)` on a path already inside an exception handler.

Both existing resize tests pinned liveness with a constant mock, which is why the
deletion passed CI invisibly; the new case is alive at the pre-probe and gone by the
time the ioctl throws.

Also adds the patch's first upstream reference: microsoft/node-pty#220 named this
mechanism in 2018 and was closed 2025-12-19 as completed after only improving the
error message; #827 is still open. Tracked as #18109.

* test(pty): release the live pty before the next fd-reuse case

The new Linux write case killed its live pty without awaiting the exit, so that
master was still open when the next case spawned. Linux hands out the lowest
free descriptor, so the retired pty got N+1 while the leaked N closed during the
400ms settle -- and `does not name a live pty foreground process off the retired
descriptor` then saw its live pty take N, failing its own reuse premise by
exactly one (`expected 22 to be 23` on CI, reproduced as 19/20 in a container).

Test isolation, not a Linux behavior difference: with the descriptor released
the case passes 8/8. Await the exit like the resize case above already does, and
switch the payload command to `sleep 1` -- the line discipline echoes a leaked
write back on its own, so nothing has to read it. Note the constraint on the
block so the next case does not repeat it.

The premise assertion stays as it is. It compares two observed descriptors, not
a hardcoded number; relaxing it to "differs from the retired one" would let both
reuse cases pass vacuously on a kernel that never reissues, which is the whole
thing they exist to demonstrate.

Verified with vitest under node:24-bookworm against a source-built patched
node-pty: 7 passed, 8 runs, no flake. Negative check in the same container --
reverting the `_close()` -> `dispose()` hook fails the case with
`expected 'leak\r\n' not to contain 'leak'`, so the rewritten assertion is still
load-bearing.

---------

Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Co-authored-by: Merge Sim <sim@local>
Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
* fix(editor): improve rich markdown table readability

Show borders and subtle striping for rich markdown tables so cell boundaries stay legible in the formatted editor.

* fix(editor): keep rich table headers sentence case

Keep the new table styling readable without forcing header text to uppercase.

* style(rich-md): fix table striping against Tiptap tbody-only DOM

Switch zebra striping to nth-child(odd) so the shaded row doesn't sit
directly under the header, and drop the compounding th font-size.

---------

Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
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