[Due for payment 2026-08-05] [Concierge] Donut chart intermittently renders blank despite valid data (flaky renderer)

[melvin-bot]

Problem

The Concierge spend-summary donut chart intermittently renders blank — the chart area shows nothing even though the underlying data is present and correct. It is not a data problem: inspecting Onyx state confirms the chart data is good, so the issue is in the chart renderer, which behaves flakily.

Steps that surface it

• Open a Concierge chat/DM that includes a donut chart (e.g. "expenses broken down by category").
• The chart may appear blank on first load.
• Re-triggering a render — leaving the chat and coming back, switching from Concierge Anywhere to the DM, or resizing the browser window — sometimes makes the chart appear.

Observed behavior

• Chart frequently renders blank on initial load.
• Same chart renders correctly after navigation/resize, confirming the data is valid and the renderer is the flaky component.
• Reproduced on both staging and prod.

Expected behavior

• The donut chart should render reliably on first load whenever valid data is available, without requiring navigation or a resize.

Related

• [Due for payment 2026-07-28] [Insights] "Top categories" donut chart labels overlap when slices are small or clustered #94613 — donut chart label overlap (related area, different defect)

Screenshots are in the originating internal Slack thread (omitted here as they may contain customer/employee data).

Reported in Slack.

Issue Owner

Current Issue Owner: @mallenexpensify

[roryabraham]

Investigation notes — mechanism identified

Reproduced locally on web (dev) with temporary instrumentation in the Victory chart renderer. Posting findings so far; still confirming the root cause of one step.

Confirmed mechanism

When the chart renders blank, <victorypie> is not a direct child of <victorychart> in the parsed tree.

Broken chart (pie at depth 4, one direct child):

victorychart > victoryaxis > victoryaxis > victorylabel > victorypie > victorylabel

Correctly rendering chart in the same report (pie at depth 1, five direct children):

victorychart > [victoryaxis, victoryaxis, victorylabel, victorypie, victorylabel]

Every element is present in both — in the broken case they are nested as a cascade instead of siblings. That single difference produces the blank chart:

1. processVictoryChartTree walks the tree recursively, so it still finds the pie at depth 4 and populates chart data with both categories.
2. resolveVictoryChartType therefore returns polar, so the chart mounts: canvas, background, title, and the centre "Total" label all render.
3. VictoryChartPolar renders category nodes from tnode.children — one level only. VictoryChartCategories never matches the pie, so VictoryChartPie is never mounted.

Result: chart chrome renders, donut does not, and nothing throws or warns.

A second variant was also observed, where the markup was cut off earlier: the pie was absent entirely, resolveVictoryChartType returned null, and BaseVictoryChartRenderer returned null — no canvas at all. Same class of failure at a different point.

Why "it's not the data" was correct but misleading

The message.html stored on the report action parses flat and renders correctly. So inspecting Onyx will always show good data, and the stored HTML cannot reproduce this. The malformed tree only appears when the chart arrives live (i.e. as the Concierge response streams in), which is why a reload fixes it — the reload parses the complete stored HTML.

This is consistent with "leaving the chat and coming back" fixing it: that unmounts and remounts, re-parsing the now-complete HTML. Instrumentation showed the broken chart commits once and never re-renders, so it cannot recover in place. Whether the reported window-resize recovery works the same way (via row recycling causing a remount) hasn't been verified.

Ruled out

• Invalid or NaN data — values are finite numbers; geometry is identical to working charts (radius: 145, innerRadius: 125, both categories present).
• Skia / CanvasKit / WebGL failure — the title and centre total are Skia Text nodes in the same canvas as the missing slices, so the canvas is painting.
• Unmeasured layout — PolarChart gates all canvas children behind hasMeasuredLayoutSize; labels painting proves it measured.
• Font-loading race — correlates, but charts render correctly with typefaces still unloaded, so it isn't causal.
• Render/remount loop — commit counts are stable.

Open question: what causes the nesting

Two candidates, needing different fixes:

1. The received markup lacks self-closing slashes on the victory tags, leaving the parser no option but to nest each element inside the previous one. That would be an upstream/serialisation problem, not a renderer bug.
2. Content-model normalisation re-parents the pie. victorypie is registered as block and victorylabel as textual in victoryHtmlElementModels.ts, so a pie landing inside a label could be hoisted by the transient render engine. That would be fixable in the element models.

Currently capturing the literal markup and the raw DOM before normalisation to separate these.

Suggested fix regardless of the above

The parser walks the tree recursively while the renderer looks exactly one level deep. That asymmetry is a latent robustness bug: any nesting yields an empty chart with no error surfaced. Making the category lookup in VictoryChartPolar recursive (mirroring processVictoryChartTree), and the same in VictoryChartCartesian which shares the one-level assumption, would render the chart correctly even from a malformed tree.

That is worth doing on its own merits, but it treats the symptom — the nesting still needs explaining, especially if hypothesis 1 turns out to be the cause.

Notes for anyone else debugging this area

Three things cost time here and are worth knowing up front:

• React DevTools cannot see inside the Skia canvas. Everything below <Canvas> is rendered by react-native-skia's own reconciler, so the component tree appears to stop at PolarChartBase even for charts that render perfectly. The console is the only usable window.
• Never log from a Skia render callback. The pie slice callback re-runs continuously; any work there (even a guarded Set lookup) will lock up the page.
• A console.log in a component body breaks React Compiler memoisation for that file, which changes the behaviour you're trying to observe. Put probes in a useEffect, and don't write refs during render.

/cc @luacmartins — happy to put up the recursive-lookup fix if that direction seems right.

[roryabraham]

Root cause confirmed: the streamed HTML is re-serialised, losing self-closing tags

Follow-up to my earlier comment. The open question there is resolved, and one claim in it was wrong — correction below.

The markup differs between the streamed and the stored copy

Instrumented the literal string handed to the HTML parser.

Persisted message.html on the report action — correct, flat:

<victoryaxis style="…" /><victoryaxis dependentaxis style="…" /><victorylabel x="32" … /><victorypie … /><victorylabel x="340" … />

Received live over Pusher while the response streams — broken, nested:

<victoryaxis style="…"><victoryaxis dependentaxis style="…"><victorylabel x="32" …><victorypie …><victorylabel x="340" …></victorylabel></victorypie></victorylabel></victoryaxis></victoryaxis>

Every self-closing slash is gone, replaced by closing tags emitted in reverse nesting order. The markup is already nested before any App code parses it.

A second difference corroborates. Attribute values that are escaped in the stored copy:

labelindicator="<ShiftedLineSegment dy="0" … />"

arrive partially decoded in the streamed copy:

labelindicator="<ShiftedLineSegment dy=&quot;0&quot; … />"

</> became raw </> while " survived. Both changes together are the signature of the HTML being parsed into a DOM and re-serialised by something that does not treat these custom tags as void/self-closing. This is a payload problem on the streaming path, not a parsing quirk in App.

Content-model normalisation is also ruled out. A probe on the raw DOM in the onElement visitor — before the transient render engine normalises anything — reports a tree identical to the post-normalisation tnode tree on every parse. victoryHtmlElementModels.ts is not involved.

Correction to my earlier comment

I said the broken chart "commits once and never re-renders". That was an artefact of my own log de-duplication and is wrong. It re-renders repeatedly — a dozen re-parses were observed as the comment streamed in. The tree never changes because the markup never improves: the chart substring is already complete and already nested in the first streamed snapshot, and stays byte-identical while the rest of the comment grows around it.

That is also the real reason navigating away and back fixes it — the remount reads the persisted flat HTML rather than the streamed value. The well-formed markup never appears during streaming at all.

What this means for the issue

• Not a flaky renderer. Given the malformed payload the outcome is deterministic. The apparent flakiness is only whether you are looking at a freshly streamed chart or a reloaded one.
• Inspecting Onyx will always show good data and well-formed HTML, because the persisted copy is pristine. That is why "it's not a data problem" was the natural reading — it was correct about the stored data and still consistent with a broken render.
• Charts that arrive live are affected; the same chart re-read from storage renders correctly.

Suggested fixes

1. Streaming path — root cause, likely backend. Stop re-serialising the chart HTML on the way to the client, or teach the serialiser that the victory tags are void. This is where the defect originates.
2. Renderer hardening — App. Independently of the above: processVictoryChartTree walks the tnode tree recursively, while VictoryChartPolar renders category nodes only from tnode.children, and VictoryChartCartesian shares the same one-level assumption. That asymmetry turns any nesting into an empty chart with nothing logged or thrown. Making the lookup recursive fixes the user-visible symptom without waiting on a backend change, and hardens the renderer against malformed input generally.

Putting up 2 now, and chasing down exactly where 1 happens.

[roryabraham]

Correction: the mangling happens in App, not on the backend

My previous comment attributed the malformed markup to the streaming/backend path. That was wrong. The HTML is mangled client-side, in this repo. Located and reproduced:

src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts

const doc = parseDocument(html);                        // htmlparser2, HTML mode, no options
...
stages.push(render(buildClippedNodes(doc, anchor)));    // dom-serializer, no options

The progressive-reveal animation parses the comment HTML into a DOM and re-serialises it once per reveal stage. Running that round trip against the installed libraries reproduces the broken payload exactly:

IN:  <victorychart width="680"><victoryaxis style="{a:1}" /><victoryaxis dependentaxis style="{b:2}" /><victorypie labelindicator="&lt;ShiftedLineSegment dy=&quot;0&quot; /&gt;" /></victorychart>

OUT: <victorychart width="680"><victoryaxis style="{a:1}"><victoryaxis dependentaxis style="{b:2}"><victorypie labelindicator="<ShiftedLineSegment dy=&quot;0&quot; />"></victorypie></victoryaxis></victoryaxis></victorychart>

Nesting cascade, self-closing slashes gone, </> decoded to raw </> while " survives — every detail of the payload in my previous comment.

Two library defaults combine to cause it:

1. htmlparser2 — recognizeSelfClosing defaults to !htmlMode, i.e. false for parseDocument. In HTML mode it deliberately ignores /> on non-void, non-foreign tags, so each <victoryaxis … /> opens an element and its siblings become its children. (Note BaseHTMLEngineProvider does pass recognizeSelfClosing: true, which is why the normal render path is unaffected — this separate tokenizer doesn't.)
2. dom-serializer@0.2.2 — carries a hardcoded void-element list (area, base, br, col, …). The victory tags aren't in it, so it emits explicit closing tags. With no decodeEntities option it also escapes only " in attribute values, leaving < and > raw — exactly the partial-decode signature.

Why it looked like a streaming problem

The input to the tokenizer is the pristine persisted HTML — usePusherDraftPacing.ts:926 does getReportActionHtml(reportAction) and hands it to startFinalRenderedHTMLReveal → tokenizeForReveal. Every reveal stage including the final one is dom-serializer output. So while the reveal bubble is on screen you see the mangled chart; once the draft clears and the real report action renders, you see the pristine one.

That is the whole "flaky renderer" behaviour: same chart, two different markup strings, depending on whether you're looking at the reveal animation or the settled report action. It also explains why Onyx always looks correct — the stored copy genuinely is.

Second call site with the same defect: src/hooks/usePendingConciergeResponse.ts:48, on the optimistic Concierge action.

Fix

Pass the options through:

const doc = parseDocument(html, {recognizeSelfClosing: true});
...
render(buildClippedNodes(doc, anchor), {selfClosingTags: true, decodeEntities: true})

One caveat from testing this: on dom-serializer@0.2.2 the output becomes <victoryaxis …></victoryaxis> rather than <victoryaxis … />. Not byte-identical to the input, but the tree is flat and correct, which is what the chart renderer needs. Worth a test asserting tokenizeForReveal(chartHtml).at(-1) re-parses to the same tree as chartHtml — that will fail today.

Also worth considering whether chart-bearing HTML should skip character-granularity tokenisation altogether, since revealing a chart progressively has no visual benefit.

Still worth doing separately

The renderer hardening from my previous comment stands on its own merits. processVictoryChartTree recurses while VictoryChartPolar and VictoryChartCartesian only look at tnode.children, so any nesting silently yields an empty chart. I have that change working locally with tests: a shared recursive lookup that collects renderable nodes at any depth without descending into an already-collected node (which matters for victorygroup, whose renderer owns its own victorybar children). Happy to raise it alongside or separately from the tokenizer fix.

A latent hazard also turned up while tracing this: Web-Expensify's validateHTML() registers the victory elements with a Flow content model rather than EMPTY, so it would nest them the same way if it were ever applied to this path. It currently isn't — flagging it so the two don't collide later.

[kavimuru]

Bug is not reproduced

96699.mp4