Claude: changes for issue #71 #90

Merged
Grandiras merged 2 commits from claude/issue-71 into main 2026-08-14 19:59:50 +00:00
Collaborator

Closes #71.

Closes #71.
Tell members what changed, in words they can read
All checks were successful
CI / build (pull_request) Successful in 32s
67b291bce6
A release was invisible from inside the app. `git log` is the technical
record and there was no human one, so a member's first sign of a new
version was a screen that had moved — the boards, the calendar, the
column filter all arrived without anybody being told.

So: `Services/Changelog.cs`, a list of releases in plain language,
newest first, and a dialog that renders it. A sparkle appears in the
header when the version on screen is one this browser has not read
about, and is gone once the dialog has been opened; "What's new" stays
in the nav menu afterwards, carrying the one place in the app that names
the running version.

Three decisions worth stating.

**One source of truth, and it is not a CHANGELOG.md.** Two lists of the
same releases drift, and the one that drifts is the one nobody has open
while writing code. The entry is therefore part of the change rather
than part of the tag: it is written in the pull request that makes the
change and dated the day it lands, which is also the only date that
exists while the code is being written. The release skill now checks
before tagging that `Changelog.Latest.Version` is the version about to
be cut.

**Copy lives beside its version, not in the translation table.**
`Translations` would grow half a dozen keys per release whose only
structure was their names. `ChangelogRelease.Changes` is keyed by locale
exactly the way `Translations` is and falls back to English by the same
rule, and `ChangelogTests` re-imposes what the translation tests impose
on everything else — both languages, nothing blank, no "handle" — plus a
jargon guard, because this is the one piece of copy in the app whose
failure mode is sounding like a commit message.

**What each browser has read is localStorage**, like the theme and the
install nudge, not a column on Member: it says nothing about the person,
and a database write per device buys nothing. `HasUnseen` compares for
equality rather than order, so a rollback also announces itself — going
quiet exactly when a member most wants to know what moved would be the
worst of the available answers.

One trap, and prefs.spec.js caught it: excluding `.nav-whatsnew` from
`.board-user button:not(.nav-hamburger)` adds a second `:not()`, which
raises that selector above `.board-user .nav-pref-options button` and
paints a surface over the theme pills — all three then read as chosen.
The round icon button overrides those defaults from its own block
instead, the way `.nav-menu-install` does.

Verified: `dotnet test` (205 passing, 9 new), and Playwright against a
live stack — the new changelog.spec.js plus prefs, install, i18n, board,
mobile, filter and boards. Screenshots taken in light, dark and German,
on desktop and at 390px.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
Collaborator

🤖 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.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/245); the reply will link the full session transcript. 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-90/run-245.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

💬 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 is v2.3.0, consistent with the release skill's new instruction to keep the top entry in step with what's about to ship.
  • The CSS specificity reasoning for not excluding .nav-whatsnew from .board-user button:not(.nav-hamburger) checks out: both selectors tie at the same specificity, so the later .nav-whatsnew block wins by source order, matching the comment's claim.
  • Changelog is reachable from NavHeader.razor and ChangelogModal.razor via the existing @using Ponente.Kanban.Services in _Imports.razor.
  • The "best-effort" JS interop pattern (try/catch around changelogUnseen/markChangelogSeen) matches the install-nudge convention already used elsewhere in this file.
  • The jargon/handle-word regex guards in ChangelogTests don't false-positive against the actual copy in Changelog.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() in prefs-interop.js, which is what production actually uses). It would be tidier for JS to just return the stored version and let Changelog.HasUnseen make the call, so there's one place that owns the comparison rule the code base is otherwise careful to avoid duplicating (see the BoardAccess.VisibleTo precedent in CLAUDE.md).


⏱ 3m 46s · 10 turns · tokens: 80.7k in / 9.1k out (+942.3k cache-read)
📜 Full session transcript

💬 **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 is `v2.3.0`, consistent with the release skill's new instruction to keep the top entry in step with what's about to ship. - The CSS specificity reasoning for not excluding `.nav-whatsnew` from `.board-user button:not(.nav-hamburger)` checks out: both selectors tie at the same specificity, so the later `.nav-whatsnew` block wins by source order, matching the comment's claim. - `Changelog` is reachable from `NavHeader.razor` and `ChangelogModal.razor` via the existing `@using Ponente.Kanban.Services` in `_Imports.razor`. - The "best-effort" JS interop pattern (try/catch around `changelogUnseen`/`markChangelogSeen`) matches the install-nudge convention already used elsewhere in this file. - The jargon/handle-word regex guards in `ChangelogTests` don't false-positive against the actual copy in `Changelog.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()` in `prefs-interop.js`, which is what production actually uses). It would be tidier for JS to just return the stored version and let `Changelog.HasUnseen` make the call, so there's one place that owns the comparison rule the code base is otherwise careful to avoid duplicating (see the `BoardAccess.VisibleTo` precedent in `CLAUDE.md`). --- ⏱ 3m 46s · 10 turns · tokens: 80.7k in / 9.1k out (+942.3k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-90/run-245.md) <!-- claude-verdict:comment --> <!-- claude-reviewed-head:67b291bce61a40146154cfa14493615e5008ed9a -->
@ -21,0 +27,4 @@
// new" — which for a first visit is true.
const CHANGELOG_KEY = 'ponente-changelog-seen';
Collaborator

changelogUnseen() reimplements the "has this browser read the newest entry" comparison in JS (localStorage.getItem(...) !== version), duplicating Changelog.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.HasUnseen is currently exercised only by ChangelogTests and 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 calling Changelog.HasUnseen(stored) from NavHeader, so the comparison rule lives in exactly one place — the same principle the code base applies to BoardAccess.VisibleTo.

`changelogUnseen()` reimplements the "has this browser read the newest entry" comparison in JS (`localStorage.getItem(...) !== version`), duplicating `Changelog.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.HasUnseen` is currently exercised only by `ChangelogTests` and 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 calling `Changelog.HasUnseen(stored)` from `NavHeader`, so the comparison rule lives in exactly one place — the same principle the code base applies to `BoardAccess.VisibleTo`.
Owner
@claude-bot
Author
Collaborator

🤖 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.

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/251); the reply will link the full session transcript. <!-- claude-run:comment --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/issue-90/run-251.md) — updating every ~5 minutes while the task runs.
Let one side decide what "unseen" means
All checks were successful
CI / build (pull_request) Successful in 38s
635bf92bcd
`changelogUnseen(version)` compared the stored version against the newest
one in JS, which is the comparison `Changelog.HasUnseen` already owns — and
documents at length, because it is deliberately an equality rather than an
ordering so a rollback also announces itself. Two copies of a rule is a rule
that gets refined on one side while the other's tests go on passing: as
written, `HasUnseen` was exercised only by `ChangelogTests` and what members
actually got came from the JS.

So the module hands back the stored string (or null) and nothing else, and
`NavHeader` asks `Changelog.HasUnseen`. `markChangelogSeen` stays where it
is; a write is storage, not a decision.

The private-mode fallback moves rather than disappears. `changelogSeenVersion`
deliberately does not catch: where localStorage is refused it throws, the
interop call fails, and NavHeader's existing best-effort `catch` leaves the
sparkle off — which is the same answer for the same reason, without a second
opinion in JS about what unseen means. That is the one thing here whose shape
changed, so changelog.spec.js gets a second test for it: a context that
refuses storage before the page's first script runs, asserting the header is
intact, the sparkle absent and the menu row still opening the dialog.

Verified: `dotnet test` (205 passing), and Playwright against a local stack —
changelog (both tests), prefs, install and i18n.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
Collaborator

Done — pushed to claude/issue-71 (635bf92), which updates PR #90.

The finding, fixed as suggested. prefs-interop.js no longer makes the "is this unseen" call:

  • changelogUnseen(version)changelogSeenVersion(), which returns the stored string or null and nothing else.
  • NavHeader passes that to Changelog.HasUnseen(seen), so the comparison the doc comment describes at length is now the one production runs and the one ChangelogTests pins — BoardAccess.VisibleTo's rule, applied here.
  • markChangelogSeen stays 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 returning null would flip it to sparkle on every load — the outcome the comment says is worse than none. So changelogSeenVersion deliberately doesn't catch: where localStorage is refused it throws, the interop call fails, and NavHeader's existing best-effort catch (JSException) leaves _changelogUnseen at its false default. 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.js rather 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 :8090 in Development):

  • dotnet build clean (the one warning is the pre-existing Calendar.razor CS8604), dotnet test205 passed.
  • Playwright against that live app: changelog.spec.js (both tests), prefs.spec.js, install.spec.js, i18n.spec.js15 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

Done — pushed to `claude/issue-71` (`635bf92`), which updates PR #90. **The finding, fixed as suggested.** `prefs-interop.js` no longer makes the "is this unseen" call: - `changelogUnseen(version)` → `changelogSeenVersion()`, which returns the stored string or `null` and nothing else. - `NavHeader` passes that to `Changelog.HasUnseen(seen)`, so the comparison the doc comment describes at length is now the one production runs *and* the one `ChangelogTests` pins — `BoardAccess.VisibleTo`'s rule, applied here. - `markChangelogSeen` stays 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 returning `null` would flip it to *sparkle on every load* — the outcome the comment says is worse than none. So `changelogSeenVersion` deliberately doesn't catch: where localStorage is refused it throws, the interop call fails, and `NavHeader`'s existing best-effort `catch (JSException)` leaves `_changelogUnseen` at its `false` default. 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.js` rather 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 `:8090` in Development): - `dotnet build` clean (the one warning is the pre-existing `Calendar.razor` CS8604), `dotnet test` — **205 passed**. - Playwright against that live app: `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](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/issue-90/run-251.md)
Author
Collaborator

🤖 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.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/252); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-90/run-252.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

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() in prefs-interop.js, what production actually used) — a duplicated rule the codebase otherwise avoids (per the BoardAccess.VisibleTo precedent). That's now resolved: prefs-interop.js's changelogSeenVersion() just returns the raw stored version (or null/throws), and NavHeader.razor calls Changelog.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-whatsnew from .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

✅ **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()` in `prefs-interop.js`, what production actually used) — a duplicated rule the codebase otherwise avoids (per the `BoardAccess.VisibleTo` precedent). That's now resolved: `prefs-interop.js`'s `changelogSeenVersion()` just returns the raw stored version (or null/throws), and `NavHeader.razor` calls `Changelog.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-whatsnew` from `.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](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-90/run-252.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:635bf92bcd355854b800a07d1df5a0aa6714089a -->
Grandiras referenced this pull request from a commit 2026-08-14 19:59:50 +00:00
Grandiras deleted branch claude/issue-71 2026-08-14 19:59:50 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/Ponente!90
No description provided.