Refetch notes on stage change so obfuscation clears - #40
Merged
Merged
Conversation
Moving a retro from reflect to group left other people's notes stuck showing reflect-stage obfuscated noise. Obfuscation is recomputed correctly per request from the retro's current status, but nothing re-fetched notes when that status changed - a per-stage remount used to do this as a side effect until "Keep the notes when the stage changes" (dabd046) removed it, and no replacement was added for the already-open board. Wire status_updated to trigger the existing reconnect refetch instead. Also fixes two more instances of the same underlying pattern found while tracing this: - dal.NoteList had no ORDER BY, so a full notes fetch (now happening on every stage change too) had no stable order. - The note_created reducer case always appended the server-confirmed note to the end instead of swapping it in at the optimistic placeholder's own position, which could reorder notes as people created them concurrently. - VotesWithCountFromModel built its result by ranging over a map, so vote-count order in discuss was non-deterministic - the classic Go map-iteration bug. Adds regression coverage at every level: dal/resources/controllers tests against real SQLite, a Vitest reducer case, and a Playwright E2E suite driving two real browser sessions through the actual reflect to group transition (verified this catches the regression: reverting the fix makes it fail showing literal obfuscated noise instead of the typed note). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…after-stage-change # Conflicts: # ui/package.json # ui/pnpm-lock.yaml
task run's data path is never created by the app itself - task dev relies on the committed ./data, and the Dockerfile does its own mkdir. CI's isolated THOUGHTS_DATA_PATH had nothing creating it, so the server exited on startup before Playwright could reach it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
Moving a retro from reflect to group left other people's notes stuck showing reflect-stage obfuscated noise. Obfuscation is recomputed correctly per request from the retro's current status, but nothing re-fetched notes when that status changed - a per-stage remount used to do this as a side effect until "Keep the notes when the stage changes" (
dabd046) removed it, and no replacement was added for the already-open board.status_updatedto trigger the existing reconnect refetch, so an open board asks again (and gets correctly de-obfuscated content) on every stage change.dal.NoteListhad noORDER BY, so a full notes fetch (now happening on every stage change too) had no stable order.note_createdreducer case always appended the server-confirmed note to the end instead of swapping it in at the optimistic placeholder's own position, which could reorder notes as people created them concurrently.VotesWithCountFromModelbuilt its result by ranging over a map, so vote-count order in discuss was non-deterministic - the classic Go map-iteration bug.Test plan
go build ./... && go vet ./... && go test -race ./...cd ui && pnpm lint && pnpm exec tsc -b && pnpm test && pnpm builddal/resources/controllerstests against real SQLite, a Vitest reducer case, and a Playwright E2E suite (ui/e2e/reflect-to-group-obfuscation.spec.ts) driving two real browser sessions through the actual reflect → group transition."n nadkvhktgwh ampapnb qnxc xul uusmaf vvagj") instead of the typed note; restored the fix and it passed cleanly across repeated runs.pnpm test:e2edocumented in CLAUDE.md.🤖 Generated with Claude Code