fix(app): show all update notice bullets - #372
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRelease-note parsing now extracts multiple bullet/numbered items from localized "App Update Notice" sections (zh targets ChangesRelease Body Parsing & Highlight Generation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the release highlight logic to extract and display all bullet points from release notes instead of just the first one. It introduces a new parsing function to handle multiple descriptions and updates the test suite to reflect these changes. A review comment points out that removing the limit on the number of highlights could lead to UI overflow and poor user experience in the release notes dialog, suggesting a reasonable upper limit of 15 highlights be maintained.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/app/src/context/highlights.test.ts (1)
96-131: ⚡ Quick winAdd a skipped-version regression for the removed highlight cap.
These assertions prove one release can emit many cards, but they never exercise the
sliceHighlightschange that removed the old 5-item limit across a release range. A multi-release payload with more than five total highlights would catch the exact skipped-version case this PR is targeting.Suggested test shape
+ test("keeps more than five highlights across skipped versions", () => { + const payload = [ + { + tag_name: "v2026.4.29", + body: "## App Update Notice\n\n- A\n- B\n- C\n", + }, + { + tag_name: "v2026.4.28", + body: "## App Update Notice\n\n- D\n- E\n- F\n", + }, + ] + + expect(loadReleaseHighlights(payload, "2026.4.29", "2026.4.27", "en")).toHaveLength(6) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/context/highlights.test.ts` around lines 96 - 131, The test currently validates localized bullets for a single release but doesn't exercise the multi-release path and the removed 5-item cap in sliceHighlights; extend or add a test that calls loadReleaseHighlights with a payload containing multiple releases (e.g., v2026.4.27, v2026.4.28, v2026.4.29) whose combined highlights exceed five items, request a range that spans them (start version earlier than the oldest and end version equal to latest) and assert that the returned highlights (from loadReleaseHighlights) include all items across releases (no slicing to 5), thus validating the sliceHighlights behavior for skipped-version scenarios.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/context/highlights.tsx`:
- Around line 96-112: parseNoticeDescriptions currently returns only lines[0]
for non-bulleted notices, which drops wrapped paragraphs; change the non-bullet
fallback to join the filtered lines into a single paragraph before trimming so a
wrapped paragraph becomes one card. Locate parseNoticeDescriptions and replace
the final return that uses lines[0] with logic that joins lines (e.g.,
lines.join(' ')), then call trimNoticeItem on that joined string (use the
existing trimNoticeItem helper) and return it as the single-element array if
non-empty.
---
Nitpick comments:
In `@packages/app/src/context/highlights.test.ts`:
- Around line 96-131: The test currently validates localized bullets for a
single release but doesn't exercise the multi-release path and the removed
5-item cap in sliceHighlights; extend or add a test that calls
loadReleaseHighlights with a payload containing multiple releases (e.g.,
v2026.4.27, v2026.4.28, v2026.4.29) whose combined highlights exceed five items,
request a range that spans them (start version earlier than the oldest and end
version equal to latest) and assert that the returned highlights (from
loadReleaseHighlights) include all items across releases (no slicing to 5), thus
validating the sliceHighlights behavior for skipped-version scenarios.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d6e5008f-f648-4dc5-a30e-13231dd5dac8
📒 Files selected for processing (2)
packages/app/src/context/highlights.test.tspackages/app/src/context/highlights.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/context/highlights.tsx`:
- Around line 105-111: The current bullets extraction treats each physical line
separately (in the block that builds bullets from lines and uses match and
trimNoticeItem), which truncates wrapped markdown bullet continuations; fix by
iterating through lines and when you find a bullet start (the existing regex
match in the bullets creation), accumulate that line plus any immediately
following lines that are continuation lines (e.g., lines that are indented or do
not start with a list marker) into a single string, then call trimNoticeItem on
the combined string and push that single item to bullets; update the logic
around the variables lines, bullets, and the match handling to skip consumed
continuation lines so you don’t double-process them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 52c69f13-df4e-4c13-9569-eac133ce14d4
📒 Files selected for processing (2)
packages/app/src/context/highlights.test.tspackages/app/src/context/highlights.tsx
Summary
Parse every bullet in the app update notice into its own release highlight instead of collapsing the notice to the first non-heading line.
Why
Chinese release notes already route through
## 中文版本and### 主要更新, but the parser only returned the first parsed line. Forv2026.4.29, that meant the in-app update dialog showed a single Chinese item while the release body had multiple user-facing updates.Related Issue
Fixes #309.
Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
Please check the parser contract: bullet notices now become multiple cards, while one-paragraph notices still stay as a single card.
Risk Notes
Low app-side parser change. The dialog already supports multiple highlights. This removes the old five-item cap so a skipped-version update can show the full parsed notice range.
How To Verify
Screenshots or Recordings
Not included. This changes parser output covered by unit tests; no new UI layout was introduced.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Tests
Improvements