Claude: changes for issue #71 #90
No reviewers
Labels
No labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Grandiras/Ponente!90
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "claude/issue-71"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #71.
🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the task runs.
💬 Verdict: comment — notes only, nothing blocking.
This is a thorough, well-tested addition (dialog, header sparkle, menu row, localStorage "seen" tracking, changelog data with EN/DE copy, unit tests enforcing tone/translation parity, and an e2e test covering the full path). I checked the pieces most likely to hide bugs and found nothing that breaks:
Changelog.Latest.Version(2.4.0) is correctly the newest, unreleased version — the latest git tag isv2.3.0, consistent with the release skill's new instruction to keep the top entry in step with what's about to ship..nav-whatsnewfrom.board-user button:not(.nav-hamburger)checks out: both selectors tie at the same specificity, so the later.nav-whatsnewblock wins by source order, matching the comment's claim.Changelogis reachable fromNavHeader.razorandChangelogModal.razorvia the existing@using Ponente.Kanban.Servicesin_Imports.razor.changelogUnseen/markChangelogSeen) matches the install-nudge convention already used elsewhere in this file.ChangelogTestsdon't false-positive against the actual copy inChangelog.cs.HasUnseen's equality-not-ordering semantics are correctly exercised by the theory test cases (older version, unknown/rolled-back version, never-seen).One minor architectural nit worth a look (non-blocking), noted inline: the "is this unseen" decision is implemented twice — once in C# (
Changelog.HasUnseen, exercised only by tests) and once independently in JS (changelogUnseen()inprefs-interop.js, which is what production actually uses). It would be tidier for JS to just return the stored version and letChangelog.HasUnseenmake the call, so there's one place that owns the comparison rule the code base is otherwise careful to avoid duplicating (see theBoardAccess.VisibleToprecedent inCLAUDE.md).⏱ 3m 46s · 10 turns · tokens: 80.7k in / 9.1k out (+942.3k cache-read)
📜 Full session transcript
@ -21,0 +27,4 @@// new" — which for a first visit is true.const CHANGELOG_KEY = 'ponente-changelog-seen';changelogUnseen()reimplements the "has this browser read the newest entry" comparison in JS (localStorage.getItem(...) !== version), duplicatingChangelog.HasUnseen(seenVersion)in C# — which per its own doc comment is deliberately a plain inequality rather than a version comparison, specifically to handle rollbacks correctly.Changelog.HasUnseenis currently exercised only byChangelogTestsand never called from production code; the actual value shown to users comes from this JS copy instead. If "unseen" semantics are ever refined, it's easy to update one side and forget the other, and the C# tests would keep passing while production behavior silently drifts from what they claim to pin. Consider having JS return the raw stored value (or null) and callingChangelog.HasUnseen(stored)fromNavHeader, so the comparison rule lives in exactly one place — the same principle the code base applies toBoardAccess.VisibleTo.@claude-bot
🤖 On it — follow along in the action run; the reply will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the task runs.
Done — pushed to
claude/issue-71(635bf92), which updates PR #90.The finding, fixed as suggested.
prefs-interop.jsno longer makes the "is this unseen" call:changelogUnseen(version)→changelogSeenVersion(), which returns the stored string ornulland nothing else.NavHeaderpasses that toChangelog.HasUnseen(seen), so the comparison the doc comment describes at length is now the one production runs and the oneChangelogTestspins —BoardAccess.VisibleTo's rule, applied here.markChangelogSeenstays in JS: a write is storage, not a decision.One thing that had to move rather than disappear. The old JS caught the private-mode throw and returned
false("nothing is new"). Returning a raw value can't express that, and returningnullwould flip it to sparkle on every load — the outcome the comment says is worse than none. SochangelogSeenVersiondeliberately doesn't catch: where localStorage is refused it throws, the interop call fails, andNavHeader's existing best-effortcatch (JSException)leaves_changelogUnseenat itsfalsedefault. Same answer, same reason, no second opinion in JS.That fallback is the only thing whose shape changed, so it gets a test — a second one in
changelog.spec.jsrather than a merge into the first, because the context has to be refusing storage before the page's first script runs, which a signed-in page can't be talked into afterwards. It asserts the header survives, the sparkle is absent, and the menu row still opens the dialog.CLAUDE.md's changelog section gained a bullet saying where the comparison lives and why the JS doesn't catch.Verified (no container engine in this runner, so I installed the .NET 10 SDK and Postgres 15 and ran the app directly on
:8090in Development):dotnet buildclean (the one warning is the pre-existingCalendar.razorCS8604),dotnet test— 205 passed.changelog.spec.js(both tests),prefs.spec.js,install.spec.js,i18n.spec.js— 15 passed, 0 failed. The refused-storage case was checked ad hoc first and then committed as the test above.⏱ 6m 21s · 40 turns · tokens: 81.0k in / 12.4k out (+3174.7k cache-read)
📜 Full session transcript
🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the task runs.
✅ Verdict: approve — no blocking issues found.
This is a re-review; the one issue raised last pass is fixed. Previously, "has this browser seen the newest changelog entry" was decided independently in both C# (
Changelog.HasUnseen, only exercised by tests) and JS (changelogUnseen()inprefs-interop.js, what production actually used) — a duplicated rule the codebase otherwise avoids (per theBoardAccess.VisibleToprecedent). That's now resolved:prefs-interop.js'schangelogSeenVersion()just returns the raw stored version (or null/throws), andNavHeader.razorcallsChangelog.HasUnseen(seen)directly to make the decision. The comparison rule now lives in exactly one place, matching the doc comments' own claims.Spot-checked the rest again:
Changelog.Latest.Version(2.4.0) is still correctly ahead of the latest tag (v2.3.0), the CSS specificity reasoning for not excluding.nav-whatsnewfrom.board-user button:not(.nav-hamburger)still holds, and the try/catch best-effort pattern around the JS interop call is intact and matches the file's own documentation. No new issues introduced by this round of commits.⏱ 0m 17s · 3 turns · tokens: 69.7k in / 1.2k out (+228.5k cache-read)
📜 Full session transcript