[Payment due @ChavdaSachin] Strip incomplete markdown from Concierge streaming drafts#89744
Conversation
During Concierge streaming, partial markdown (e.g. unclosed bold, links, code blocks) was being rendered as raw syntax by ExpensiMark. Add stripIncompleteMarkdown() to sanitize the tail of bodyMarkdown before parsing, removing unclosed constructs so they don't flash briefly in the UI. Co-authored-by: Issa Nimaga <inimaga@users.noreply.github.com>
melvin-bot
Bot
requested review from
ChavdaSachin and
trjExpensify
and removed request for
a team
|
@ChavdaSachin Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
melvin-bot
Bot
requested review from
a team and
dukenv0307
and removed request for
a team
|
@dukenv0307 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
🚧 @inimaga has triggered a test Expensify/App build. You can view the workflow run here. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a283fbbdef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
The failing checks are not related to this PR's code changes:
All substantive checks pass: ESLint, typecheck, all 8 test jobs, prettier, spellcheck, React Compiler, storybook, perf-tests, and builds. Recommendation: Re-run the |
This comment has been minimized.
This comment has been minimized.
|
@dukenv0307, does this need my review? |
|
@ChavdaSachin Hold off on reviewing this for now. |
inimaga
changed the title
|
@cretadn22 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@inimaga any update here? |
# Conflicts: # src/pages/inbox/conciergeDraftState.ts
|
@ChavdaSachin This is ready to go. Could you please re-review. If you're to re-perform testing, you need to do so using a test account with the |
|
🚧 @inimaga has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
WebScreen.Recording.2026-05-19.at.4.31.49.PM.mov</details? |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@ahmedGaber93 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Co-authored-by: Issa Nimaga <inimaga@users.noreply.github.com>
| break; | ||
| } | ||
|
|
||
| codeRanges = getCodeRanges(result).ranges; |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The strip-then-normalize sequence for bold (lines 172-175) and strikethrough (lines 177-180) is identical apart from the delimiter and its single-character replacement. This 4-line block is duplicated verbatim.
Extract a helper that performs both steps for a given delimiter:
function stripAndNormalizeDelimiter(text: string, delimiter: string, replacement: string, codeRanges: TextRange[]): {result: string; codeRanges: TextRange[]} {
let updatedRanges = getCodeRanges(text).ranges;
const stripped = stripUnpairedLastLineDelimiter(text, delimiter, updatedRanges);
updatedRanges = getCodeRanges(stripped).ranges;
const normalized = normalizeDelimiterForExpensiMark(stripped, delimiter, replacement, updatedRanges);
return {result: normalized, codeRanges: getCodeRanges(normalized).ranges};
}Then the two blocks collapse to:
({result, codeRanges} = stripAndNormalizeDelimiter(result, BOLD_DELIMITER, '*', codeRanges));
({result, codeRanges} = stripAndNormalizeDelimiter(result, STRIKETHROUGH_DELIMITER, '~', codeRanges));Reviewed at: 583714b | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
🚧 @inimaga has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/inimaga in version: 9.3.78-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes required. This PR adds a |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.78-1 🚀
|
|
🤖 Payment issue created: #91351 |
5 participants
Explanation of Change
During Concierge streaming,
bodyMarkdownarrives via Pusher before the server-renderedfinalRenderedHTMLis available. Some streamed markdown is incomplete at chunk boundaries (for example,**boldwithout the closing**, or[text](urlwithout the closing)) and complete emphasis can also briefly leak raw delimiters when a streaming snapshot still contains Markdown-style text such as*Workspaces*.This PR adds a
stripIncompleteMarkdown()sanitization step inbuildConciergeDraftReportActionbeforegetParsedComment(). It removes trailing incomplete markdown constructs: unclosed links/images, bold (**), strikethrough (~~), code blocks (```), and inline code (`), so half-finished syntax is hidden until a later streaming chunk completes it.For completed streamed emphasis, it normalizes
**bold**and~~strike~~outside code ranges to ExpensiMark single-delimiter syntax (*bold*and~strike~) before parsing. It also parses Markdown emphasis that arrives through streamingfinalRenderedHTMLsnapshots, including plain text or HTML fragments with text nodes like*Workspaces*. This keeps the word styled during streaming while preventing raw*,**, or~~delimiters from flashing in the UI.The final saved message is unaffected since it uses server-side HTML rendering via
finalRenderedHTML.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/633323
Tests
npm test -- --runTestsByPath tests/unit/pages/inbox/conciergeDraftState.test.ts --runInBand --no-watchmanOffline tests
N/A — this change only affects the client-side rendering of streaming Concierge draft messages, which require an active connection.
QA Steps
Current result in Staging
Expected result
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Resolved.md.rendering.during.streaming.mp4