feat(24): World History Chronicle — 编年史面板#14
Merged
Conversation
Add append-only world_history table with idempotent genesis seed, scoped public list/detail GET, worker writeback + worldHistorySync, and web 编年史 drawer (filter, 太乙纪 pagination, minutes modal). Includes list perf (SWR cache, slim SQL) and CONTRACTS C-07b. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 20 minutes and 25 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds shared world-history schemas and storage, genesis seeding, public/internal game-server routes, Colyseus sync handling, and a new chronicle drawer with list and minutes modal UI. ChangesWorld history chronicle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (2)
apps/web/src/hooks/useWorldHistory.test.ts (1)
67-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a paginated sync regression case.
These tests only cover page 1 with room to prepend. Please add cases for
page > 1andentries.length === pageSizeso the helper can't regress on later-page insertion or page-size overflow again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/hooks/useWorldHistory.test.ts` around lines 67 - 99, The `mergeEntryIntoPageState` tests only cover first-page prepend behavior, so add regression cases that exercise later pages and full pages: verify the helper’s behavior when `page > 1` and when `entries.length === pageSize`, especially that a matching entry is inserted or queued without breaking pagination or overflowing the current page. Use the existing `mergeEntryIntoPageState`, `emptyPageState`, and `genesisEntry` helpers in `useWorldHistory.test.ts` to cover both scenarios and assert the resulting `next`, `inserted`, and `isNew` state.apps/game-server/src/world/world-history-seed.test.ts (1)
89-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the partial-seed retry path.
The current idempotency test only exercises 0→3→3. It will still pass if a failed first attempt leaves 1-2 genesis rows and a retry duplicates the whole set. Add a regression that starts from a partially seeded room and asserts the total stays at 3.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/game-server/src/world/world-history-seed.test.ts` around lines 89 - 99, The idempotency coverage in seedWorldHistoryIfNeeded is incomplete because it only verifies the 0→3→3 path. Add a regression in world-history-seed.test.ts that starts with a partially seeded room, then calls seedWorldHistoryIfNeeded again and asserts the final genesis count remains exactly 3. Use the existing helpers countGenesisEntries and listWorldHistory to validate that a retry after a partial first attempt does not duplicate genesis rows.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/game-server/src/routes/internal-world-history.ts`:
- Around line 85-86: Reject cross-room broadcast targets by ensuring the fan-out
target matches the persisted room identity in internal-world-history handling.
In the route logic around the `mapRoomId`/`roomId` assignment and the sync
broadcast path, stop using a body-supplied room override for notification
routing; instead, always broadcast to the owning `roomId` (or explicitly
validate that `data.mapRoomId` matches `roomId` and fail if it does not). Apply
the same correction in both affected sync paths so the `internal-world-history`
write and Colyseus notification stay aligned.
- Around line 89-112: The World History append flow in internal-world-history
should not fail the request after insertWorldHistoryEntry has already committed.
Move broadcastWorldHistorySync(mapRoomId, entry) into a non-fatal path (or wrap
it separately) so a broadcast error does not trigger the catch block and a retry
duplicate append; keep the successful res.json({ ok: true, entry }) response
tied to the insert success in the handler.
In `@apps/game-server/src/world/world-history-repository.ts`:
- Around line 345-357: The default gameYear is being chosen from all rows before
status filtering, which can select a year with no matching rows for the
requested status. Update listMemory to apply filterRowsByStatus first, then
derive availableYearsFromRows and the fallback gameYear from that
status-filtered set; keep the same ordering and pagination logic for the
yearRows selection.
- Around line 314-329: The insertWorldHistoryEntry flow currently accepts
free-input yesCount/noCount even for vote_minutes, which can leave persisted
tallies inconsistent with minutes.ballots. Update the world-history write path
in insertWorldHistoryEntry and the row builders it calls
(insertSqlRow/insertMemoryRow or shared mapping) to derive yesCount/noCount from
ballots or reject mismatched counts, and ensure genesis entries still persist
both counts as null.
- Around line 113-120: The persistence fallback in getSql() is silently
degrading to the in-memory Map when DATABASE_URL is missing, which hides data
loss; change WorldHistoryRepository so the normal path fails fast when
persistence is unavailable. Update getSql() to require DATABASE_URL for
production use, and keep the Map-backed path only behind an explicit test-only
or opt-in switch. Make sure the repository methods that rely on getSql() surface
a clear error instead of continuing with non-persistent storage.
In `@apps/game-server/src/world/world-history-seed.ts`:
- Around line 57-78: The genesis seeding logic in world-history-seed.ts only
short-circuits when countGenesisEntries(roomId) is already at 3, so partially
seeded rooms or concurrent retries can append duplicate genesis rows. Update the
seeding flow around countGenesisEntries, GENESIS_ROWS, and
insertWorldHistoryEntry to either insert only the missing genesis identities
based on what already exists, or add a transactional/uniqueness guard so the
room always ends with exactly three genesis entries.
In `@apps/web/src/ChatPage.tsx`:
- Around line 661-671: The toast queue is keying visibility off toast.kind plus
toast.title in ChatPage, which causes distinct queued toasts with the same title
to share one identity and prevents the effect from rerunning after dismissal.
Update the toastIdentity logic to use a per-toast unique identifier from the
toast object instead of title-based fields, and keep the useEffect tied to that
unique identity so each queued toast re-triggers visible state and timeout
handling independently.
In `@apps/web/src/components/WorldHistoryPanel.tsx`:
- Around line 150-156: The click handler in WorldHistoryPanel’s detail-loading
path does not handle rejected onFetchEntryDetail calls, which can leave
detailLoading stuck true and trigger an unhandled rejection. Update the async
onClick flow around onFetchEntryDetail(entry.id) to use error handling (for
example, try/catch/finally) so detailLoading is always reset even when the fetch
fails, and optionally surface or log the error instead of letting the rejection
escape.
In `@apps/web/src/hooks/useWorldHistory.ts`:
- Around line 84-101: The paginated slice in useWorldHistory should not be
mutated by blindly prepending live sync entries, because that only works for the
first page and can overflow later pages while leaving pagination metadata stale.
Update the insertion logic around shouldMergeEntryIntoVisibleList and the
returned entries array so local insertion only happens in safe cases (for
example, the active first page), otherwise avoid inserting into prev.entries and
trigger a refetch of the current query/page after sync.
In `@docs/CONTRACTS.md`:
- Line 146: The reset contract has a C-05 cross-reference mismatch: the Reset
row in CONTRACTS.md says `world_history` survives reset and references C-05, but
the current C-05 text describes `collective_events` being deleted, so make the
contract consistent by updating the C-05 entry and/or the Reset row to use the
same room-shared data term and reset behavior. Locate the relevant `Reset`
contract row and the C-05 collective-events section in CONTRACTS.md, then align
the wording so the surviving/deleted data is described identically in both
places.
In `@packages/shared/src/worldHistory.ts`:
- Around line 97-106: The world history validation in
validateWorldHistoryStrings only checks title and proposal, so blocked content
can still slip through via nested minutes fields. Update this helper to also
inspect the WorldHistoryMinutes payload used by the detail/sync path, including
minutes.proposalFull, vote reasonZh, and signatory text, or otherwise ensure
those displayed minutes strings are derived from already-validated fields. Use
the existing validateWorldHistoryStrings and WorldHistoryMinutes symbols to
locate the affected validation flow.
---
Nitpick comments:
In `@apps/game-server/src/world/world-history-seed.test.ts`:
- Around line 89-99: The idempotency coverage in seedWorldHistoryIfNeeded is
incomplete because it only verifies the 0→3→3 path. Add a regression in
world-history-seed.test.ts that starts with a partially seeded room, then calls
seedWorldHistoryIfNeeded again and asserts the final genesis count remains
exactly 3. Use the existing helpers countGenesisEntries and listWorldHistory to
validate that a retry after a partial first attempt does not duplicate genesis
rows.
In `@apps/web/src/hooks/useWorldHistory.test.ts`:
- Around line 67-99: The `mergeEntryIntoPageState` tests only cover first-page
prepend behavior, so add regression cases that exercise later pages and full
pages: verify the helper’s behavior when `page > 1` and when `entries.length ===
pageSize`, especially that a matching entry is inserted or queued without
breaking pagination or overflowing the current page. Use the existing
`mergeEntryIntoPageState`, `emptyPageState`, and `genesisEntry` helpers in
`useWorldHistory.test.ts` to cover both scenarios and assert the resulting
`next`, `inserted`, and `isNew` state.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 57f8350b-fe2a-44b3-8806-4df4645a7d15
📒 Files selected for processing (30)
apps/game-server/src/index.test.tsapps/game-server/src/index.tsapps/game-server/src/room/store.tsapps/game-server/src/routes/internal-world-history.tsapps/game-server/src/routes/world-history.tsapps/game-server/src/world/world-history-broadcast.tsapps/game-server/src/world/world-history-repository.test.tsapps/game-server/src/world/world-history-repository.tsapps/game-server/src/world/world-history-seed.test.tsapps/game-server/src/world/world-history-seed.tsapps/web/src/ChatPage.tsxapps/web/src/components/DialogueBar.tsxapps/web/src/components/ShellDrawer.test.tsapps/web/src/components/ShellDrawer.tsxapps/web/src/components/WorldHistoryMinutesModal.test.tsapps/web/src/components/WorldHistoryMinutesModal.tsxapps/web/src/components/WorldHistoryPanel.test.tsapps/web/src/components/WorldHistoryPanel.tsxapps/web/src/hooks/useColyseusRoom.tsapps/web/src/hooks/useWorldHistory.test.tsapps/web/src/hooks/useWorldHistory.tsapps/web/src/index.cssdocs/CONTRACTS.mdpackages/npc-memory/migrations/0008_world_history.sqlpackages/npc-memory/migrations/meta/_journal.jsonpackages/shared/src/aetherNexusLore.tspackages/shared/src/colyseus.tspackages/shared/src/index.tspackages/shared/src/worldHistory.test.tspackages/shared/src/worldHistory.ts
Comment on lines
+113
to
+120
| function getSql(): ReturnType<typeof getSharedSql> | null { | ||
| const url = process.env.DATABASE_URL; | ||
| if (!url) return null; | ||
| if (!sqlClient) { | ||
| sqlClient = getSharedSql(url); | ||
| } | ||
| return sqlClient; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail fast when persistence is unavailable.
If DATABASE_URL is missing, this repository silently falls back to the process-local Map. That makes the chronicle appear healthy while dropping every entry on restart, which breaks the append-only persistence this feature is introducing. Keep the in-memory path test-only, or require an explicit opt-in instead of auto-downgrading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/game-server/src/world/world-history-repository.ts` around lines 113 -
120, The persistence fallback in getSql() is silently degrading to the in-memory
Map when DATABASE_URL is missing, which hides data loss; change
WorldHistoryRepository so the normal path fails fast when persistence is
unavailable. Update getSql() to require DATABASE_URL for production use, and
keep the Map-backed path only behind an explicit test-only or opt-in switch.
Make sure the repository methods that rely on getSql() surface a clear error
instead of continuing with non-persistent storage.
Comment on lines
+314
to
+329
| export async function insertWorldHistoryEntry( | ||
| input: InsertWorldHistoryEntryInput, | ||
| ): Promise<WorldHistoryPublicEntry> { | ||
| const blocked = validateWorldHistoryStrings({ | ||
| title: input.title, | ||
| proposal: input.proposal, | ||
| }); | ||
| if (blocked) { | ||
| throw new Error(`world history content blocked: ${blocked}`); | ||
| } | ||
|
|
||
| parseWorldHistoryMinutes(input.minutes); | ||
|
|
||
| const sql = getSql(); | ||
| const row = sql ? await insertSqlRow(input) : await insertMemoryRow(input); | ||
| return toPublicEntry(row); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate or derive vote tallies from the ballots.
yesCount/noCount are stored as free input even when minutes.kind === "vote_minutes". The new repo test already inserts 12 ballots with an 8/4 split while persisting 8–3, so list/detail can expose contradictory results. Derive the counts from minutes.ballots or reject mismatches, and keep both counts null for genesis rows.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/game-server/src/world/world-history-repository.ts` around lines 314 -
329, The insertWorldHistoryEntry flow currently accepts free-input
yesCount/noCount even for vote_minutes, which can leave persisted tallies
inconsistent with minutes.ballots. Update the world-history write path in
insertWorldHistoryEntry and the row builders it calls
(insertSqlRow/insertMemoryRow or shared mapping) to derive yesCount/noCount from
ballots or reject mismatched counts, and ensure genesis entries still persist
both counts as null.
Comment on lines
+345
to
+357
| async function listMemory(params: ListWorldHistoryParams): Promise<ListWorldHistoryResult> { | ||
| const pageSize = clampPageSize(params.pageSize); | ||
| const page = normalizePage(params.page); | ||
| const status = params.status ?? "accepted"; | ||
|
|
||
| const allRows = memoryRowsForRoom(params.roomId); | ||
| const years = availableYearsFromRows(allRows); | ||
| const gameYear = params.gameYear ?? years[0] ?? 1; | ||
|
|
||
| const yearRows = filterRowsByStatus( | ||
| allRows.filter((row) => row.gameYear === gameYear), | ||
| status, | ||
| ).sort((a, b) => b.sequence - a.sequence); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Choose the default year from the filtered set.
Both paths pick gameYear from all rows before applying status. If the latest year has only accepted entries, status="rejected" returns an empty first page even when older rejected rows exist. Filter by status before computing availableYears and the default year.
Also applies to: 384-392
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/game-server/src/world/world-history-repository.ts` around lines 345 -
357, The default gameYear is being chosen from all rows before status filtering,
which can select a year with no matching rows for the requested status. Update
listMemory to apply filterRowsByStatus first, then derive availableYearsFromRows
and the fallback gameYear from that status-filtered set; keep the same ordering
and pagination logic for the yearRows selection.
Comment on lines
+150
to
+156
| onClick={() => { | ||
| void (async () => { | ||
| setDetailLoading(true); | ||
| const full = await onFetchEntryDetail(entry.id); | ||
| setDetailLoading(false); | ||
| if (full) setSelectedEntry(full); | ||
| })(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle rejected detail fetches in the click path.
onFetchEntryDetail() can reject on network errors, and this handler currently leaves detailLoading stuck true while also surfacing an unhandled promise rejection.
Proposed fix
onClick={() => {
void (async () => {
setDetailLoading(true);
- const full = await onFetchEntryDetail(entry.id);
- setDetailLoading(false);
- if (full) setSelectedEntry(full);
+ try {
+ const full = await onFetchEntryDetail(entry.id);
+ if (full) setSelectedEntry(full);
+ } catch {
+ // keep the panel usable on transient fetch failures
+ } finally {
+ setDetailLoading(false);
+ }
})();
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onClick={() => { | |
| void (async () => { | |
| setDetailLoading(true); | |
| const full = await onFetchEntryDetail(entry.id); | |
| setDetailLoading(false); | |
| if (full) setSelectedEntry(full); | |
| })(); | |
| onClick={() => { | |
| void (async () => { | |
| setDetailLoading(true); | |
| try { | |
| const full = await onFetchEntryDetail(entry.id); | |
| if (full) setSelectedEntry(full); | |
| } catch { | |
| // keep the panel usable on transient fetch failures | |
| } finally { | |
| setDetailLoading(false); | |
| } | |
| })(); | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/WorldHistoryPanel.tsx` around lines 150 - 156, The
click handler in WorldHistoryPanel’s detail-loading path does not handle
rejected onFetchEntryDetail calls, which can leave detailLoading stuck true and
trigger an unhandled rejection. Update the async onClick flow around
onFetchEntryDetail(entry.id) to use error handling (for example,
try/catch/finally) so detailLoading is always reset even when the fetch fails,
and optionally surface or log the error instead of letting the rejection escape.
Comment on lines
+97
to
+106
| /** Returns first blocklist failure reason, or null if title and proposal pass. */ | ||
| export function validateWorldHistoryStrings(fields: { | ||
| title: string; | ||
| proposal: string; | ||
| }): string | null { | ||
| for (const text of [fields.title, fields.proposal] as const) { | ||
| const result = checkPlayerMessageContent(text); | ||
| if (!result.allowed) return result.reason; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate the nested minutes strings too.
This helper only guards title and proposal, but the public detail/sync path also exposes minutes.proposalFull, vote reasonZh, and signatory text. A worker can therefore pass a benign top-level payload and still persist blocked content inside minutes. Expand the validator to walk the WorldHistoryMinutes payload, or derive the displayed minutes text from already-guarded fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/shared/src/worldHistory.ts` around lines 97 - 106, The world history
validation in validateWorldHistoryStrings only checks title and proposal, so
blocked content can still slip through via nested minutes fields. Update this
helper to also inspect the WorldHistoryMinutes payload used by the detail/sync
path, including minutes.proposalFull, vote reasonZh, and signatory text, or
otherwise ensure those displayed minutes strings are derived from
already-validated fields. Use the existing validateWorldHistoryStrings and
WorldHistoryMinutes symbols to locate the affected validation flow.
Isolate broadcast failures from persist success, validate mapRoomId, fix toast identity and pagination merge, and skip duplicate genesis titles on partial seed retry. Co-authored-by: Cursor <cursoragent@cursor.com>
6 tasks
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
1 participant
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
world_historypersistence (migration0008) with idempotent 3× genesis seed from classicalaetherNexusLoreGET /rooms/:roomId/world-history(list, nominutes) +GET /:entryId(detail); internal workerPOSTwriteback withworldHistorySyncbroadcastdocs/CONTRACTS.mdC-07bVerification
pnpm --filter @aetherlife/game-server test— 213 passedpnpm --filter @aetherlife/web test— 121 passedpnpm --filter @aetherlife/shared test— 209 passedpnpm --filter @aetherlife/npc-memory db:migrateon deploy (new0008_world_historytable)Test plan
pnpm dev:stack→ open drawer → 编年史 → 3 genesis entries under 已采纳GET /rooms/default/world-historyreturns 403 without room membershipMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Tests