Repository navigation
workflow: a node prompt and a step reply cut at their bound keep the character there - #2533
Merged
Merged
Conversation
Contributor
Author
|
@Birfy review request for the current tip.
|
…r in half cleanLine has counted its bound in code points since ThinkFlowLab#1588, but cleanPrompt and readStepResult still used String#slice, so a cut inside one astral character left a lone surrogate: half a character in the prompt a node is told to follow, and half a character in the reply body a step hands on. The brief a run receives goes through the same cut. Both lines now use the code-point cut cleanLine already documents. Nothing else changes: no bound, no payload shape, no new abstraction. Docs-not-needed: the bounds and what they mean are unchanged; only the unit they are counted in is fixed.
Collaborator
|
Verdict: PASS at d9284be Ran at this tip:
|
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
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.
Fixes #2429
Owner: Dev-5
Written-by: Dev-5
Reviewer: Birfy
Risk tier: T2 — the source line sits under
plugins/**; the rest is its test and one changelog fragment.Base: 90a5944
What changed
plugins/workflow/src/graph.js— the two cuts that still usedString#slicenow count code points:cleanPrompt(a node prompt, and the task briefworkflow/assignnormalises withMAX_BRIEF) and the step body kept byreadStepResult.cleanLineis not touched; its own cut already did this since fix(workflow): keep Runs and timeline task titles whole instead of cutting UTF-16 units #1588, and those are the two bounds the issue names.cleanPromptreturnedout…trim().slice(0, max)andreadStepResultusedkept.join('\n').trim().slice(0, 2000). A bound landing between the halves of a surrogate pair stored the lone high surrogate —\ud83d— which every encoder reads as U+FFFD and every renderer draws as�. The bound also counted UTF-16 units, so a prompt of 4000 emoji was cut at 2000 characters.cleanLine's own comment above it states — "Cut on code points, never on UTF-16 units: a unit offset can split a surrogate pair and leave a lone surrogate, which renders as a replacement character."plugins/workflow/src/service.js,core/client.jsandpatches/ds-bot/*are untouched.Why these two sites and not a new rule
cleanLinesits in the same file and already follows the code-point rule; so do the two earlier fixes of this class, #1993/#2000 for the host and #2018 for the workflow node avatar.cleanPromptandreadStepResultwere the two bounds left behind, which is why the issue names exactlygraph.js:37and:81. The change is pinned to those two lines —cleanLine's shape (text.length <= max ? text : Array.from(text).slice(0, max).join('')) inlined at each, no new abstraction and no refactor of the callers.Files (
git diff --numstat 90a59447...HEAD)plugins/workflow/src/graph.js+5/−2plugins/workflow/test/graph.test.js+23/−2changelog.d/workflow-prompt-codepoint-cut.fixed.md+1/−0Docs-not-needed: the bounds and what they mean are unchanged; only the unit they are counted in changed.Red first
On the clean base
90a594471b5886737b433e116f6397aa8ca2b6f2with only the new case added — the source untouched — inplugins/workflow:# tests 9 / # pass 8 / # fail 1:The case builds a prompt of 3999
athen one🚀and asserts the bounded text equals the prompt (the🚀fits, counted as one character) and carries no lone surrogate; it then askscleanPromptfor a text one character over the bound and asserts the🚀drops whole, and finally feeds 1999athen🚀throughreadStepResult. Each assertion is red on the base and green after the change. The same file at the tip is# tests 9 / # pass 9 / # fail 0.Ran
plugins/workflow, at the tip,node --import ./test/resolve.mjs --test test/graph.test.js→# tests 9 / # pass 9 / # fail 0plugins/workflow, at the tip,npm test→# tests 441 / # suites 4 / # pass 441 / # fail 0TZ=UTC npm --prefix core test→ exit 0,# tests 2927 / # pass 2927 / # fail 0node scripts/lint-plugins.mjs→ exit 0,Found 16 warnings and 0 errors(the warnings are the existing ones inplugins/social,plugins/timelineandplugins/tree),pack-importOK for every plugincore/index.jsorcore/src/host/**change;ds-bot-patches generate/check— nocore/orpatches/change.Docs
Docs-not-needed:the cut bound and its maximum stay the same number of characters; only the unit it is counted in moves from UTF-16 to code points, which is whatcleanLinealready documents in this file and what the two earlier fixes of this class established.Checkpoint
Test advisory: PASS at
d9284be993cd79bb14c0461143ca3d0a491b56fb— FlyBot Test, independent of the author: red first reproduced on a clean90a59447(9 tests / 8 pass / 1 fail, the failure isnot ok 9,'…\ud83d'vs'…🚀'); after the change 9/9,plugins/workflow441/441,core2927/2927;cleanLinebyte-identical between base and tip, two hunks ingraph.js,+5/−2. Advisory: this is a T2 change, so the merge still waits on a Verdict naming this tip.{ "pr": 2533, "claim": "Dev-5", "issue": 2429, "summary": "The node prompt and the step body still cut their text with String#slice, so a bound inside a surrogate pair stored half a character, and an emoji-heavy prompt only reached half the room. Both cuts now count code points, the rule cleanLine has followed since #1588; cleanLine itself, every bound and every caller stay as they were. The brief a run receives goes through the same cut, so it also stops being truncated mid-character. Nothing outside plugins/workflow/src/graph.js, its test and one changelog fragment moves.", "files": [ "plugins/workflow/src/graph.js", "plugins/workflow/test/graph.test.js", "changelog.d/workflow-prompt-codepoint-cut.fixed.md" ], "checks": { "ran": "plugins/workflow npm test; TZ=UTC npm --prefix core test; node scripts/lint-plugins.mjs", "result": "green", "at": "d9284be993cd79bb14c0461143ca3d0a491b56fb" }, "next": "none \u2014 ready for review" }