Repository navigation
Conversation
The folded row names its reasoning in the summary (ThinkFlowLab#1624); opened, the body prints that same line again. Assert the open state hides the peek and that the shipped client bundle carries the rule, so ThinkFlowLab#1636 cannot land half-generated.
The folded row names a line of its reasoning inside the summary (ThinkFlowLab#1624) and the body below prints the whole text; with no open-state rule the truncated copy stayed above that same line, so an opened row read the sentence twice. One rule hides the preview once the details element is open, and the closed state renders exactly as before. core/client.js is rebuilt from core/src/client with `npm run build`, and patches/ds-bot output comes from `node scripts/ds-bot-patches.mjs generate`; both are generated artifacts, committed here together with their source. Docs-not-needed: one CSS rule in CONVERSATION_CSS hides the folded row's preview line while the row is opened; docs/subsystems/core-client.md describes the client's structure, not the thinking row's markup, which it did not cover before either.
|
@Birfy — review request for PR #1637 at the current tip Scope:
As of my read: 0 reviews, 0 comments on this PR; the tip above is unchanged since it was pushed. Checks on this tip are green apart from the Verdict gate. |
|
Verdict: PASS at 5d32f0e Ran at this tip: Reviewed the live head (fork HugoXK/FlyBot, fork exemption for The rule is the right shape: Checks at Not run: the full |
…inkFlowLab#1643) Fixes ThinkFlowLab#1642 Written-by: Dev-1 Owner: Dev Reviewer: Reviewer Tip: 0835e88 Tree: a5b971e ## What happens The folded thinking row prints one line of its reasoning in its summary (ThinkFlowLab#1624) — in the folded state it is the row's only text, so it has to be worth reading. It took whatever line came first: a fence (```` ```js ````) or a horizontal rule (`---`, `***`, `___`) stood in for the thought; a clip at 160 UTF-16 units could halve a surrogate pair, which renders as U+FFFD; and a clipped line gave no sign it had been clipped. ## What the preview does now `peekLine` in `core/src/client/cells.js`, inside `AssistantCell`'s folded row only: - it skips lines that are only markup — a fence (three or more backticks or tildes, with or without a language) and a horizontal rule — and takes the first line that has prose; - it cuts on grapheme boundaries (`Intl.Segmenter` where the platform has it, code points otherwise, so never inside a surrogate pair) and appends `…` when it cuts, inside the same 160-unit cap: 159 units plus the ellipsis. An uncut line is unchanged; - when every line is markup it answers a line count (`{n} line(s) of reasoning`), never a fence fragment and never an empty frame — `ThinkFlowLab#1624`'s rule that the folder row never reads as an empty frame still holds; - the two fallback strings go through the host translator and have zh entries in core's dictionary (`{n} 行思考内容`), so the fallback is not hard-coded English. The row's structure, the open/closed caret behaviour, `turn-reasoning.js`, the `ThinkFlowLab#1636` open-state rule in `styles.js` and the live-running guard are untouched. ## Red → green The first commit of this PR is the tests only, `973678e50fdc8fd7656674b67205666527052453` (tree `67b71a65…`), and stays in history as its own commit. On it `core/test/thinking-row.test.js` is 9 tests / 6 pass / 3 fail, exactly the three new cases; the five `ThinkFlowLab#1624` cases and `ThinkFlowLab#1636`'s open-row case pass untouched: - `ThinkFlowLab#1642: a fence or a rule is not the peek line; the first prose line is` — the peek was ` ```js ` and then `---`; - `ThinkFlowLab#1642: a cut preview keeps whole characters and says it was cut` — `'a'.repeat(159) + '😀' + ' tail'` peeked as 160 units ending in the lone surrogate `d83d`, with no ellipsis; - `ThinkFlowLab#1642: reasoning that is only markup gets a neutral summary, never a bare fence or an empty row` — the peek was ` ```js `. On this tip the file is 9 tests / 9 pass / 0 fail. The existing `ThinkFlowLab#1624: the peek line is capped` case still pins 160 characters and still holds: a cut line is 159 units plus `…`. A rebase onto the new main, `1ed614a7 → c26089f` (which merged `ThinkFlowLab#1637`, a change to `core/client.js` and to the `0003`/`0008` patches), rewrote both commits and force-updated the branch, so the PASS written at `d0a516ec` no longer applies. It conflicted in `core/test/thinking-row.test.js` and in the generator's `patches/ds-bot/files/test/thinking-row.test.js` mirror: the test conflict kept both sides (`ThinkFlowLab#1636`'s open-state case and this PR's three cases), and the generated files were taken back from main and regenerated (`npm --prefix core run build`, then `node scripts/ds-bot-patches.mjs generate`) rather than hand-merged — `ds-bot-patches check` reports `core/ equals ds-bot/ + patches/ds-bot/`. The red commit was re-run on the new base and fails the same three cases. New tip `0835e882…`, tree `a5b971eb…`; the change set is the same nine files. ## Generated artifacts `core/client.js` is the committed bundle rebuilt from `core/src/client` with `npm --prefix core run build` (the dictionary lives in the bundle). `patches/ds-bot/0001-core-fixes.patch` (`src/client/i18n.js`), `patches/ds-bot/0003-flybot-api-client.patch` (`src/client/cells.js`) and `patches/ds-bot/0008-client-bundle.patch` come from `node scripts/ds-bot-patches.mjs generate`, which rewrote no other patch; `patches/ds-bot/files/test/thinking-row.test.js` is the generator's mirror of the test. No generated file was edited by hand. ## Checks run `TZ=UTC ARCH_DOCS_BASE=c26089f5 npm run verify` → **exit 0** on this tip: - core 1927 / social 535 / tree 125 / timeline 129 / workflow 234 / `.github` 549 / interop 37 / live-e2e 24 tests, **fail 0** in every suite; - `build-client --check` clean; - `arch-docs: OK (9 changed file(s))` — baseline ref `ARCH_DOCS_BASE=c26089f5`, the branch base; - `ds-bot-patches check`: `ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)`, `10 patches apply cleanly in order`, `core/ equals ds-bot/ + patches/ds-bot/`; - `changelog.mjs check`: 15 fragments well formed; - `lint-plugins`: 0 errors (warnings only, unchanged from main). At the previous tip one full run had one failure that this diff cannot reach: `core/test/connector-access.test.js:492` (`a friend's line later in the turn is noted too`) waits a fixed 20 ms and went red once under the parallel suite; that file then ran 24/24 on its own and the full verify passed on the rerun. That flake did not appear in the run above at this tip, and nothing in `core/test/connector-access.test.js` is touched by this diff. `Docs-not-needed: the change stays inside AssistantCell's folded preview and adds two keys to the existing zh map; docs/subsystems/core-client.md describes the client's structure and the plugin copy that map carries, not the thinking row's preview line, which the page did not describe before ThinkFlowLab#1624 either.` ## Checkpoint <!-- flybot:checkpoint --> { "pr": 1643, "claim": "Dev", "issue": 1642, "summary": "The folded thinking row printed whatever line came first, so a fence or a horizontal rule could stand in for the thought, a clip at 160 UTF-16 units could halve an emoji into a lone surrogate, and a clipped line gave no sign it was clipped. The preview now skips markup and takes the first line with prose, cuts on grapheme boundaries with an ellipsis inside the same 160-unit cap, and falls back to a line count when the reasoning is only markup. Two dictionary keys carry that fallback in en and zh, and the five ThinkFlowLab#1624 cases stay green untouched.", "files": ["core/src/client/cells.js", "core/src/client/i18n.js", "core/test/thinking-row.test.js", "core/client.js", "patches/ds-bot/0001-core-fixes.patch", "patches/ds-bot/0003-flybot-api-client.patch", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/test/thinking-row.test.js", "changelog.d/1642-folded-thinking-preview.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=c26089f5 npm run verify", "result": "green", "at": "0835e88201fba7cecbe4249a4a9c5a32b2efdd23" }, "next": "none — ready for review" } <!-- /flybot:checkpoint -->
Fixes #1636
Written-by: Dev-1
Owner: Dev
Tip: 5d32f0e
Tree: 316eed218b9140c7a6ce120f22d6073342831496
What happens
ThinkingRow(core/src/client/cells.js:142-145) keeps a one-line preview of the reasoning inside<summary>, which is what makes a folded row name what it holds (#1624).detailscarries noopenstate andCONVERSATION_CSShad no open-state rule for.bt-think-peek— only.bt-think[open] .bt-think-caret— so opening the row left the truncated grey copy of the first line directly above the full text: the same sentence printed twice.The fix, one rule
core/src/client/styles.js(CONVERSATION_CSS), right after the peek's own rule:The closed state is byte-identical: the peek is still rendered into the summary exactly as before, and only the open state hides it.
cells.js, the empty guard, "a live step draws no row",turn-reasoning.jsand the peek's wording/charset (that is P-129) are untouched.Red → green
The first commit of this PR is the test only,
773af6f3f7f8c06c8f39b844fbb9d42c2091c6ea, and stays in history as its own commit. On itcore/test/thinking-row.test.jsis 6 tests / 5 pass / 1 fail:The five
#1624cases pass on that tip unchanged, and they still pass on this tip (peek text, 160-char cap, body text, live step draws no row, whitespace-only draws no row) — the closed state is pinned, not just claimed.On this tip (green commit
5d32f0e26a4c3b0a0e9c7870dda81672e732190d) the file is 6 tests / 6 pass / 0 fail.The test collects the
.bt-think[open] …rules fromCONVERSATION_CSSand requires one to hide.bt-think-peek, and requires the committedcore/client.jsto carry the same rule. Asserting on the shipped CSS is the shapecore/test/face-people.test.jsalready uses for a client rule; with a CSS-only fix the rendered tree still carries the peek in the summary, so the open state is observable through the rule that hides it, not through adetailsattribute.Two files appear twice, by the generator's shape
core/is generated asds-bot/+patches/ds-bot/.core/test/thinking-row.test.js(new in #1631, not an upstream file) is mirrored topatches/ds-bot/files/test/thinking-row.test.js;core/src/client/styles.jsis an upstream file mapped to0003-flybot-api-client, so its change shows up inside0003-flybot-api-client.patch.core/client.jsis the committed bundle and is regenerated into0008-client-bundle.patch.Generated artifacts in this diff, and how they were produced:
core/client.js—npm --prefix core run build(core/scripts/build-client.mjs);verifyruns--checkon it;patches/ds-bot/0003-flybot-api-client.patch,patches/ds-bot/0008-client-bundle.patch,patches/ds-bot/files/test/thinking-row.test.js—node scripts/ds-bot-patches.mjs generate. Only those three changed; the other patches the generator rewrites came out byte-identical to the base.Why each of the seven files moves
One line each, so the size of the diff is not a question:
core/src/client/styles.js— the rule itself, the comment above it, nothing else.core/client.js— the committed bundle, rebuilt bynpm --prefix core run build; never hand-edited, andverify'sbuild-client --checkfails if it drifts fromsrc/client.core/test/thinking-row.test.js— the new open-state case; the five#1624cases in the same file are untouched.patches/ds-bot/0003-flybot-api-client.patch— regenerated:src/client/styles.jsis an upstream file owned by topic0003.patches/ds-bot/0008-client-bundle.patch— regenerated: it carriescore/client.js.patches/ds-bot/files/test/thinking-row.test.js— the generator's mirror of the test file (not an upstream file, so it goes tofiles/).changelog.d/1636-thinking-row-duplicate-line.fixed.md— the user-facing line.core/src/client/cells.jsis not touched: the fix is the CSS rule only, and the closed state renders byte-identically because the markup is unchanged.Both regenerated patches were produced on this tree and are byte-identical to a fresh
generateon it; the other patches the generator rewrites came out identical to the base.The test asserts on the
CONVERSATION_CSSstring, not on a toggleddetailsattribute: with a CSS-only fix the rendered tree still carries the peek inside the summary in every state, so the open state is only observable through the rule that hides it (the same shapecore/test/face-people.test.jsuses for a client CSS rule). The bundle assertion keeps the generatedcore/client.jsfrom going stale.Checks run
TZ=UTC ARCH_DOCS_BASE=331dd620 npm run verify→ exit 0 on this tip:.github549 / interop 37 / live-e2e 24 tests, fail 0 in every suite;build-client --checkclean (the committed bundle matchessrc/client);arch-docs: OK (7 changed file(s))— baseline refARCH_DOCS_BASE=331dd620, the branch base;ds-bot-patches check:ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5),10 patches apply cleanly in order,core/ equals ds-bot/ + patches/ds-bot/;Docs-not-needed: one CSS rule in CONVERSATION_CSS hides the folded row's preview line while the row is opened; docs/subsystems/core-client.md describes the client's structure, not the thinking row's markup, which it did not cover before either.Not run: GitHub Actions. This gate is the local verify above plus the patch and bundle fingerprints; no empty commit was pushed to poke CI.
Checkpoint
{
"pr": 1637,
"claim": "Dev",
"issue": 1636,
"summary": "An opened thinking row printed its first line twice: the preview lives in the summary (#1624) and nothing hid it in the open state. One CSS rule hides the preview when the details element is open; the closed state is unchanged. The test pins the open-state rule and the rebuilt bundle carrying it, and the five closed-state cases stay green.",
"files": ["core/src/client/styles.js", "core/client.js", "core/test/thinking-row.test.js", "patches/ds-bot/0003-flybot-api-client.patch", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/test/thinking-row.test.js", "changelog.d/1636-thinking-row-duplicate-line.fixed.md"],
"checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=331dd620 npm run verify", "result": "green", "at": "5d32f0e26a4c3b0a0e9c7870dda81672e732190d" },
"next": "none — ready for review"
}