Skip to content

fix(frontend): AI narrative duplicates on back/forward under Cache Components - #59

Merged
Shaan-alpha merged 8 commits into
mainfrom
fix/activity-narrative-duplication
Jul 31, 2026
Merged

fix(frontend): AI narrative duplicates on back/forward under Cache Components#59
Shaan-alpha merged 8 commits into
mainfrom
fix/activity-narrative-duplication

Conversation

@Shaan-alpha

Copy link
Copy Markdown
Owner

Shaan reported that analysing shaan-alpha, navigating away, and coming back doubled the AI roast text. A second round trip tripled it.

Root cause

Not the roast logic — cacheComponents: true in next.config.ts (added in v0.8.6 for /share/[slug] ISR) changed navigation semantics repo-wide, and nothing in our code accounted for it.

Next 16 no longer unmounts a route on navigation. It hides it behind React's <Activity> (display: none) and keeps up to 3 routes alive. From Next's own guide (node_modules/next/dist/docs/01-app/02-guides/preserving-ui-state.md):

Effects run on every hide-to-visible transition, not just the initial mount.

So useState survived the hide while the effect tore down and re-ran on the re-show. NarrativeStream appended off that surviving state, and the backend narrative cache (app/narrative/service.py:72) replays the whole narrative as one chunk on a cache hit — so each round trip appended one more complete copy.

The key on <NarrativeStream> does not help: same key, so Activity preserves that exact instance.

Changes

narrative-stream.tsx — accumulate into a per-connection local buffer and setText(buffer) instead of setText(prev => prev + chunk); reset text/status when a real stream starts; record the finished stream id in a useRef (refs survive hide/show) so a re-show of a completed narrative returns early and never reopens the SSE connection.

Chose the ref guard over simply resetting text on every effect run — both stop the duplication, but the guard also skips a pointless backend round trip and the re-fade of text the user is already looking at. The per-connection buffer is kept as well: it is what makes a stream interrupted mid-flight restart cleanly rather than resume on top of a partial.

results-view.tsx — same root cause, different symptom, committed separately. trackAnalyzeSubmitted re-fired on every re-show, inflating analyze_submitted in PostHog (measured 4 events for 1 real analysis after three round trips). Guarded with a ref keyed on username:generated_at; a genuinely new or refreshed report still fires.

Verification

Both fixes written test-first, each confirmed failing first against the unfixed code with the exact reported signature:

  • narrative: expected 2 to be 1 after one round trip, expected 4 to be 1 after three
  • analytics: expected 1, got 4

New tests drive a real React <Activity> hide-to-show cycle rather than mocking the behaviour.

vitest run 77 passed / 25 files, tsc --noEmit clean, eslint src clean, next build clean (14/14 static pages, route table unchanged).

Not in this PR

Recorded under CHANGELOG [Unreleased] rather than minting v1.0.11 — the version bump ritual (4 constants) and the tag/release are operator calls.

A follow-up audit of every useEffect and prop-seeded useState in frontend/src reproduced 3 more Activity hazards, all unfixed and documented in docs/PROGRESS_LOG.md:

  1. history-grid.tsx:12useState(analyses) goes stale. Highest value of the three: a newly-saved analysis never appears in /me after a hide-to-show cycle. Unreachable before Cache Components, because the remount reseeded it.
  2. badge-row.tsx — a pinned-open popover survives the route being hidden. Popover.Portal mounts outside the Activity boundary, so display: none never reaches it. Narrow reachable path (browser back while pinned); wants a real-browser confirm.
  3. badge-row.tsx:15 — the badge trigger is not exposed as a button. Unrelated to Activity. The comment claims Popover.Trigger renders a <button>; it renders a <span> with role: null. Base UI warns about it at runtime.

Also noted: backend/pyproject.toml is on 1.0.3 while the other three version constants are on 1.0.10 — the v1.0.7 drift guard only asserts package.json == APP_VERSION, so that pair drifted unnoticed.

Next 16's cacheComponents hides a route behind React's <Activity> instead
of unmounting it. useState survives the hide while effects tear down and
re-run on the re-show, so the stream appended onto text it had already
accumulated -- and the backend narrative cache replays the whole narrative
as a single chunk on a hit. Each back-then-forward round trip stacked one
more complete copy of the roast.

Accumulate into a per-connection buffer instead of off the previous
render's state, reset text/status when a real stream starts, and record
the finished stream id in a ref (refs survive hide/show) so a re-show of a
completed narrative returns early rather than reopening the connection.
…-show

Same root cause as the previous commit. Because every effect re-runs on
each Activity hide->show transition, one browser back-then-forward counted
as another analysis in PostHog -- measured 4 analyze_submitted events for
1 real analysis after three round trips.

Guard on a ref keyed by username:generated_at. Refs survive the
transitions, so the event fires once per analysis while a genuinely new or
refreshed report still reports.
Changelog entries go under [Unreleased] -- the version bump ritual and the
tag/release stay operator calls. The progress log carries the root cause,
why the ref guard was chosen over resetting state on every effect run, and
the three unfixed Activity hazards the follow-up audit reproduced.
Copilot AI review requested due to automatic review settings July 31, 2026 02:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

All CHANGELOG/PROGRESS_LOG entries for the three open PRs are consolidated
here so the branches do not conflict on these two files. Records the merge
order (#60 first -- it is what makes CI green) and which audit findings
remain unfixed.
Records the finding that matters beyond this repo: the useLayoutEffect
cleanup reset the Next guide prescribes does not reach portaled content,
because the update lands on a subtree React has just hidden and hidden
Activity content re-renders at low priority.
Browser verification showed the popover Activity leak is not a real bug --
main already reports painted:false after browser-back. The fix was
reverted in #62 and the user-facing changelog line is removed rather than
left claiming a fix for something that was never broken.

Also records both process lessons: the first CDP run used Page.navigate
(a full document load) and proved nothing, and a jsdom-only failure is not
evidence of a product bug when portals, layout or paint are involved.
Shaan-alpha and others added 2 commits July 31, 2026 09:03
Three of the four entries on the verify skill's known-noise console list
were real bugs. Also removes a duplicated changelog bullet introduced when
the withdrawn finding-2 line was edited out.
@Shaan-alpha
Shaan-alpha merged commit c08afed into main Jul 31, 2026
5 checks passed
@Shaan-alpha
Shaan-alpha deleted the fix/activity-narrative-duplication branch July 31, 2026 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants