Code Review

Overview

Minimal, targeted bug fix: parseSummaryData now coerces non-string array elements via JSON.stringify before they reach parseSummaryElement, which assumes string input and crashes with .indexOf is not a function otherwise. The coercion point is correct — normalise at the boundary, not in the parser.

Issues / Suggestions

1. JSON.stringify(undefined) is not a string

JSON.stringify(undefined) returns undefined (not "undefined"), so a sparse or undefined-containing array still injects a non-string and reproduces the crash. Defensively:

if (Array.isArray(summary))
  return summary.map((e) => (typeof e === 'string' ? e : (JSON.stringify(e) ?? '')));

This can't come from to_jsonb today, but is a cheap guard worth keeping.

2. Unnecessary allocations in the common (all-strings) path

Since parseSummaryData results are cached by _summaryCache, the map only runs once per row, so this isn't a hot-path concern. But if you wanted to avoid the allocation when all elements are already strings:

if (Array.isArray(summary))
  return summary.every((e) => typeof e === 'string')
    ? (summary as string[])
    : summary.map((e) => (typeof e === 'string' ? e : (JSON.stringify(e) ?? '')));

Not required given the cache, but worth knowing.

3. Style nit: redundant parentheses

The wrapping (...) around the ternary in .map((e) => (typeof e === 'string' ? e : JSON.stringify(e))) add no precedence value here — the arrow body has no ambiguity. Minor.

4. Comment justification

The comment explains a real upstream workaround (TF to_jsonb re-parsing), which is exactly the kind of non-obvious WHY worth keeping. No notes there.

Summary

The fix is correct and well-scoped. The only actionable item is guarding the JSON.stringify(undefined) → undefined edge case (suggestion 1). Suggestions 2–3 are optional polish.

Code Review

Overview

Minimal, targeted fix: coerces non-string elements in the summary array before they reach parseSummaryElement (which calls .indexOf on its argument and would throw on an object). The root cause is TimeFusion's to_jsonb re-parsing JSON-looking text[] elements into objects.

Issues / Suggestions

1. Redundant outer parentheses (style)

?? has higher precedence than the ternary ?:, so the expression already parses as intended without the outer parens:

// current — outer parens are no-ops
summary.map((e) => (typeof e === 'string' ? e : JSON.stringify(e) ?? ''))

// simpler
summary.map(e => typeof e === 'string' ? e : JSON.stringify(e) ?? '')

2. The ?? '' fallback is only reachable for undefined/Symbol/function elements

JSON.stringify returns undefined (not a string) only when the value is undefined, a Symbol, or a function — not for the plain objects that to_jsonb produces. For the stated bug the ?? is dead code. It's harmless to keep as a belt-and-suspenders guard, but the comment could mention this so the next reader doesn't wonder whether there's a serialization failure path they're missing.

3. No regression test

There are existing test files (web-components/test/log-worker.test.ts). A small unit test covering the before/after behaviour would prevent silent regression (the error manifests as a rendering failure, not a thrown exception visible in CI):

it('parseSummaryData coerces non-string elements', () => {
  // simulate what to_jsonb produces: an object instead of a string
  const result = parseSummaryData([..., [{ key: 'val' }, 'plain string']]);
  expect(result).toEqual(['{"key":"val"}', 'plain string']);
});

4. Upstream issue reference (minor)

Linking to the upstream fix (monoscope-tech/timefusion#53) in the comment would make it easier to clean this guard up once the TF binary is fully rolled out.

Summary

The fix is correct and the blast radius is perfectly minimal (+3 / -1). Points 1–2 are stylistic; the only substantive ask is a regression test (point 3).

Code Review

Overall: Correct, minimal fix. The JSON.stringify coercion is the right choice — String(e) would give [object Object] for re-parsed JSON objects, whereas JSON.stringify faithfully reproduces the original element text.

Succinctness nit

The outer parentheses wrapping the ternary are redundant. Because ?? binds tighter than ?: in JS, JSON.stringify(e) ?? '' already groups as the else-branch without them:

// current (outer parens redundant)
summary.map((e) => (typeof e === 'string' ? e : JSON.stringify(e) ?? ''))

// tighter
summary.map(e => typeof e === 'string' ? e : JSON.stringify(e) ?? '')

Comment verbosity

The 2-line comment can be collapsed to one line; the upstream issue number carries the key context:

// Guard against to_jsonb re-parsing text[] elements (monoscope-tech/timefusion#53).

Minor correctness note

JSON.stringify(null) returns "null" (a string), so the ?? '' fallback only fires for undefined, Symbol, or Function values — none of which appear in a JSON payload. The guard is harmless but its actual coverage is narrower than it looks.

No blocking issues — the fix is correct and well-scoped.