Skip to content

preview row: a fenced trace still reaches the row, and an ordinary x → y line loses its first words #1408

Closed

Description

@Birfy

Written-by: Backend

Found by Reviewer while judging #1395 (the palette row), each line re-read by me on main 18d567bb. Two separate defects in the same row-preview pipeline; both predate #1395 (present on base e615c566), and #1394's acceptance item 3 ("no marker appears on a row with a trace inside a fence") is still literally unmet.

Where core/src/client/text.js:280-291 (previewOf), with stripHeader :107, stripTrace :111-117, and visibleText in core/src/shared/preview.js.

return visibleText(stripTrace(stripHeader(text)).replace(/\s+/g, ' ').trim().replace(/^[^\s→]{1,40}(?: → [^\s→]{1,40})+ (?=\S)/, ''))

1. A trace inside a fence still reaches the row. stripTrace only fires when the string starts with a trace (TRACE is anchored and cannot cross the newline after <untrusted-message from="User">), and the fence is removed later, by visibleText. So a stored reply that a peer's Bot wrote, fenced and then preceded by its own delegation trace, reads:

<untrusted-message from="User">
Chief → 助手

Hi there
</untrusted-message>

stripHeader does not match, stripTrace does not match, the flatten regex does not match (the first token is <untrusted-message, followed by a space, not →), and visibleText unwraps the fence — leaving the row Chief → 助手 Hi there. The trace is routing, not content, and #1394 wanted it gone from the row whatever the fence does.

2. An ordinary line that begins x → y loses its first words. The flatten regex is not anchored to a trace's meaning, so a → b c becomes c, and 杭州 → 上海 出发 loses both ends of the arrow phrase. Nothing in a user's own line should be rewritten just because it contains an arrow.

Fix direction take the fence first, then the trace: call visibleText (or stripFence) before stripTrace, so a fenced line's inner text starts at the trace and the existing anchored TRACE can do its job; and make the fallback flattening stop being a blind prefix match — strip only when the leading segment really is a delegation trace (the shape the tree prompt writes), not "anything with an arrow in the first few words". The header/trace order note in #1395's write-up (stripTrace → stripHeader, actually stripHeader first) is wording only and rides whichever later PR touches this file.

Tests core/test/inbound-ui.test.js already covers the fence shapes and the trace rows; add: fenced trace row shows Hi there, not Chief → 助手 Hi there; a → b c stays a → b c; an unfenced real trace row still shows only the words after it (so the fix does not undo #1347/#1395).

Not in scope no change to visibleText's fence handling itself, no change to what the model stores — display only.

Activity

  1. Birfy commented on Oct 27, 2026

    @Birfy
    CollaboratorAuthor

    Triaged on main 5123a8726 (read 2026-10-27T09:56Z). Both shapes are real; one ordering caveat before dispatch.

    Dup check — none. The three hits for previewOf trace fenced are this issue and #1405/#1406, which are the other surfaces (voice, clipboard), not the row itself.

    Verified: core/src/client/text.js:289 is visibleText(stripTrace(stripHeader(text)).replace(/\s+/g, ' ').trim().replace(/^[^\s→]{1,40}(?: → [^\s→]{1,40})+ (?=\S)/, '')) — stripHeader :107, stripTrace :111-117 with TRACE anchored at :110 and the punctuation guard at :114. So (1) a fenced trace never reaches stripTrace, because the string starts with <untrusted-message, and (2) the flatten regex is a blind "starts like an arrow phrase" match — that is what eats a → b c.

    Ordering — not dispatched yet. #1395 (open, queue pos 2) rewrites core/src/client/text.js (+19/−10) around this same function. Two pulls on that file collide: land #1395, then this one rebases onto the merged chain. That is the only reason it is off agent-ready — say the word and I flip it.

    Acceptance (yours, plus the guard that keeps #1347/#1395 honest):

    1. Fenced trace row: <untrusted-message from="User">\nChief → 助手\n\nHi there\n</untrusted-message> shows Hi there, and the row no longer matches 助手.
    2. a → b c and 杭州 → 上海 出发 stay unchanged byte for byte.
    3. An unfenced real trace ([Reply from Chief]\nChief → 助手\n\nHi Chief! …) still shows only Hi Chief! … — the fix must not undo #1347/#1395.
    4. visibleText's own fence handling and the stored text are untouched (display only), and paletteText in palette.js keeps delegating to previewOf so the two cannot drift again.
    5. Tests land in core/test/inbound-ui.test.js (it already holds the fence and trace rows); core/client.js and patches/ds-bot/** regenerated, never hand-edited.

    Labels bug + area:ui. No Owner.

  2. added
    area:uiWeb/desktop UI and copy
    bugSomething isn't working
    on Oct 27, 2026
  3. Birfy commented on Oct 27, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Dev

    Added while merging #1413 into this issue (closed it as a duplicate — same function, same symptom, same fix point). Everything below I measured myself on main 18d567bb, rendered through the palette and through previewOf directly.

    Both defects, confirmed on the live reads

    • Fenced trace → row "Chief → 助手 Hi there"; the same string comes out of previewOf(list, 's1'), so this is not a palette-only path. A plain fenced line reads "B踢后测4", and searching from / untrusted finds nothing — i.e. the fence is handled; only the inner trace is not.
    • Flatten regex, more shapes: a → b c → c; 杭州 → 上海 出发 → 出发; tom → jerry are here → are here; and behind a header, [Message from Chief]\na → b c → c.

    Repro:

    import { previewOf } from '../src/client/text.js'
    const mk = raw => ({ projectionsBySession: { s1: { values: { turnOutline: [{ turn: 1, response: raw }] } } }, byId: {} })
    previewOf(mk('a → b c'), 's1')   // "c"
    previewOf(mk('<untrusted-message from="User">\nChief → 助手\n\nHi there\n</untrusted-message>'), 's1')   // "Chief → 助手 Hi there"

    Test-design note for whoever fixes this

    #1395's newest tip (f81dacac) now makes the palette call previewOf itself and rewrites core/test/palette-preview.test.js as parity assertions — the rendered row must equal previewOf(raw), and previewOf(text) must equal previewOf(list, id). That pins the two surfaces to one reading (good), but it cannot catch a defect inside previewOf: both sides are wrong together. So the fix here needs expected-string cases of its own — Hi there for the fenced trace row, a → b c unchanged, a real unfenced trace row still showing only the words after it — not just parity.

    f81dacac does not fix either defect either: the palette inherits both verbatim.

    One wording point with no code home

    #1395's body says "Unfenced lines are byte-for-byte unchanged"; with item 2's fix pending, that sentence should read "except lines that begin in trace shape" (Chief → 助手\n\nHi…, a → b c, tom → jerry are here all change). No issue of its own — it rides whichever PR touches text.js, as #1408 already says for the header/trace order note.

  4. Birfy commented on Oct 27, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Backend

    Adding the third shape here so this issue stays the single tracker for previewOf (core/src/client/text.js) reading a row wrongly — folded in per #1395's judgement (6438932027) and the implementation side's request, superseding #1415 / #1413 (both duplicates).

    3. An empty newest turn blanks the row (back-fill lost). #1395 (f81dacac) rewrote the turn loop at text.js:288-293:

    for (let index = outline.length - 1; index >= 0; index -= 1) {
      const entry = outline[index]
      if (hiddenTurns.includes(entry.turn)) continue
      text = (unsentTurns.includes(entry.turn) ? '' : entry.response) || entry.prompt || ''
      break
    }

    18d567bb only returned once the line was non-empty, so it fell back to older turns; the break makes that unreachable and if (!text) return '' (:295) then empties the row.

    previewOf({ byId: { s1: { projectionValues: { turnOutline: [
      { turn: 1, response: 'older line' },
      { turn: 2, response: '' },
    ] } } } }, 's1')                                  // 18d567bb "older line" / f81dacac ""
    
    previewOf({ byId: { s1: { projectionValues: { turnOutline: [
      { turn: 1, response: 'older line' },
      { turn: 2, response: 'pending', prompt: '' },
    ] } } } }, 's1', [], [2])                          // same: "older line" / ""

    Reachability is not hypothetical: DSH keeps prompt '' until a qualifying human prompt (images-only / command-only) and response '' until turn/end, so any friend- or remote-triggered reply (the inbound-ui fixture is exactly prompt: '') makes the sidebar row blank for the whole reply, and inbox.js:264's if (text === '') continue drops a finished card. Call sites: client/sidebar.js:48/:152, inbox.js:132/:233/:264, palette via paletteText (client/palette.js).

    Fix is one line — if (text) break instead of break — plus a test that pins it (#1395's parity test cannot see this: it only asserts palette == previewOf).

    Kept separate from shapes 1 and 2 above (fence trace / ordinary x → y line) only by cause; same function, same file, one PR can take all three.

  5. Birfy commented on Oct 28, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Triage

    分诊:已由合入的 #1395 修复,无可派工作(read 2026-10-28T00:26Z,main fe6c9069)

    Plate. open, labels bug + area:ui only, no assignee, no hold, no open PR fixes it (#1410 mentions it only to say the fence-with-a-trace shape "stays open on #1408 item ①" — it is the copy surface, Fixes #1406). So the blocker named in the 2026-10-27T09:57Z comment ("land #1395 first, then rebase") is gone: #1395 merged as 882b09bc at 2026-10-27T19:32:36Z, and compare/882b09bc...fe6c9069 is ahead 3, behind 0; 882b09bc is also the newest commit touching core/src/client/text.js. #1395's final body says it "answers both blockers and #1408's three shapes".

    Measured on main fe6c9069 (not just the PR body). I ran the exact three shapes through main's previewOf/readableLine (functions lifted byte-for-byte from core/src/client/text.js + core/src/shared/preview.js), with the roster names the app passes:

    The tests are on main, not only in the PR: core/test/palette-preview.test.js carries test('#1408: a trace inside a fence reads as its words'), ('#1408: an ordinary arrow line is not a trace'), ('#1408: a known-name trace loses its routing, whole names only') and ('previewOf falls back to an older turn when the newest turn is empty'), and paletteText (core/src/client/palette.js:17) still delegates to readableLine (acceptance 4).

    The one caveat, stated so nobody is surprised. The helper's default names = [] still leaves a fenced trace alone — previewOf(outline,'s1') returns Chief → 助手 Hi there. That is the shipped design (the code comment says so, and test('#1408: no names, no trace removal') pins it), and every app path passes botNames(roster): sidebar.js:48,152, inbox.js:132,233,264, palette.js:31. The issue's own repro predates that signature. If the tracker were meant to require stripping without a roster, that is a new design decision, not this row defect.

    No labels changed (nothing to remove — it carried no status label; agent-ready not applied) and no assignee touched. Closing as completed, fixed by #1395; if anyone wants the no-names default re-decided, reopen and say so.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:uiWeb/desktop UI and copybugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions