Slice/m2.4#351
Open
failerko wants to merge 44 commits into
Open
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
filter×sort×search pipeline; favorite-first is Layer 0 of the comparator (floats within any filter, not a separate filter pass); archived rows appear only under the Archived filter; nulls-last for last-opened; case-insensitive search across title, description, genre label, and tags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exposes storiesStore (read selectors + open-failure slot) and hydrateStories/rehydrateStories via lib/stores. apply stays internal; only the hydrate thunk is public, matching the appSettingsStore pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Remove "used by Y" comment from rehydrateStories (violates comment discipline) - Remove redundant StoryRow type re-export from stories.ts (not re-exported from stores/index.ts; all imports come directly from @/lib/db) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Non-delta config-table writes for story operational state: setStoryFavorite, setStoryArchived (rejects drafts), touchStoryOpened, openStory (resolves branch, touches, sets navigation store, calls navigate callback). Each transaction followed by rehydrateStories. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Story owns its branch-scoped graph; references() carry no onDelete so the cascade is fully manual child->parent. vault_calendars (shared) and assets blobs (content-addressed, GC-owned) are excluded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
onEditInfo/onDuplicate/onExport are now optional; overflow menu hides each item when its callback is absent. M2 wires only Archive/Unarchive + Delete; the three deferred items return in M4.4/M6.6/M9.4. All hardcoded user-facing strings converted to t() against the new storyCard locale block in common.json. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Play-test queries now resolve names via t() so locale renames are caught at the source. MODE_LABEL_KEY replaces the inline ternary; satisfies Record<StoryMode,string> restores exhaustiveness for future modes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Persistent warn bar for app-level alerts (e.g. AI not configured). Non-dismissible; state controls dismiss via removal. Warn-tinted via bg-warning opacity-[.12] overlay + border-warning border-b, mirroring collision-list-row + save-bar patterns. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drafts cannot be archived (data-model gates archive on status='active'). The Archive/Unarchive overflow item now renders only when !story.isDraft. Delete and favorite remain ungated. Adds DraftHidesArchive play story to assert the item is absent; updates the per-state table in the pattern doc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
M2.4 replaced EmptyState with StoryList, retiring emptyTitle/emptyBody in favor of list.welcomeTitle/welcomeBody. Remove the orphaned keys and repoint the i18n test to a still-live landing key. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Plain View under ScreenShell does not scroll on native; cards below the viewport fold were unreachable. Wraps the list in ScrollView with contentContainerClassName="flex-grow" so the empty-state centering (flex-1 items-center justify-center) still fills the viewport. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collapse the bespoke StoryCardVM into the canonical stories row plus the two genuinely-derived display strings (lastOpenedRelative, chapterLabel); the per-field transforms move to the card. Date-agnostic contract preserved: display strings still computed in the selector. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
StoryCard now takes StoryCardData (the stories row plus the two derived display strings) and derives favorited/archived/isDraft/genreLabel/mode from the row, reading definition through a loose cast since drafts carry partial JSON. Story-list, dev route, and stories rebuilt on row-shaped fixtures; title moves to Compounds/Story/StoryCard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
StoryCardData = Story row + lastOpenedRelative + chapterLabel; the card derives favorited/archived/isDraft/genreLabel/mode from the row while the two display strings stay pre-formatted in the selector. Inventory folder and Storybook title updated to components/story/; archived-draft is now impossible (status is a single lifecycle column). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
last_opened_at was seconds while the rest of the app uses Date.now() ms; align to ms and drop the UI-threaded clock so actions self-serve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the db-agnostic thunk hydrate + the dead readStoriesRows/hydrateStories exports; rehydrate now reads+applies inline. No payoff to the seam for stories (no boot composition, barrel pulls db anyway). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…9496d) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughImplements Slice 2.4 (story list as a real surface): adds a Zustand stories store with rehydration, ChangesStory List Surface + Stories Store
Sequence Diagram(s)sequenceDiagram
participant User
participant IndexScreen
participant selectStoryCards
participant storiesStore
participant openStory
participant setStoryFavorite
participant deleteStory
participant SQLiteDB
rect rgba(100, 149, 237, 0.5)
note over IndexScreen,storiesStore: Mount / hydration
IndexScreen->>SQLiteDB: rehydrateStories(db)
SQLiteDB-->>storiesStore: applyRows(rows)
storiesStore-->>IndexScreen: StoryRow[]
IndexScreen->>selectStoryCards: selectStoryCards(rows, query, nowMs)
selectStoryCards-->>IndexScreen: StoryCardData[]
end
rect rgba(144, 238, 144, 0.5)
note over User,openStory: Open story
User->>IndexScreen: tap card
IndexScreen->>openStory: openStory(id, ctx, navigate)
openStory->>SQLiteDB: select currentBranchId
alt draft (no branch)
openStory-->>IndexScreen: { status: 'no-branch' }
IndexScreen->>IndexScreen: show ConcurrentStatePrompt
else has branch
openStory->>SQLiteDB: update lastOpenedAt
openStory-->>IndexScreen: { status: 'ok', branchId }
IndexScreen->>IndexScreen: navigate to reader/composer
end
end
rect rgba(255, 165, 0, 0.5)
note over User,deleteStory: Delete story
User->>IndexScreen: tap delete → confirm AlertDialog
IndexScreen->>deleteStory: deleteStory(id, ctx)
deleteStory->>SQLiteDB: DELETE branch-scoped rows + branches + story (transaction)
deleteStory->>storiesStore: rehydrateStories(db)
storiesStore-->>IndexScreen: updated StoryCardData[]
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request replaces the temporary landing page with a fully featured story list and redesigned story cards, supporting search, filtering, sorting, and transactional story deletion. The review feedback highlights opportunities to improve error handling during story deletion, safely handle missing stories when archiving, optimize navigation snappiness by running touchStoryOpened asynchronously, and avoid potential sorting issues in the comparator by using 0 instead of -Infinity as a fallback timestamp.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
lib/actions/stories/delete-story.ts (1)
29-45: ⚡ Quick winAlign the cascade comment with the actual table list.
The comment says assets are excluded, but
entryAssetsis included inBRANCH_SCOPED. Please update either the comment or the list so future edits don’t rely on contradictory guidance.🤖 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 `@lib/actions/stories/delete-story.ts` around lines 29 - 45, The comment in the BRANCH_SCOPED declaration states that assets are excluded as they are handled separately, but entryAssets is actually included in the array. Update the comment to accurately reflect the current table list by clarifying which specific assets (if any) are excluded from BRANCH_SCOPED, or remove entryAssets from the array if it should not be included. Ensure the comment and the actual list are consistent so future developers have correct guidance.
🤖 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 `@app/index.tsx`:
- Around line 88-90: The onArchiveToggle handler calls setStoryArchived without
checking if the row is in draft status, but setStoryArchived rejects draft
archival by design. Since this is a fire-and-forget call (void), the rejection
becomes unhandled. Add a guard condition before calling setStoryArchived to
check if row?.status is 'draft' and prevent the archive toggle from executing
when the row is in draft state, ensuring the call only proceeds for non-draft
rows.
In `@components/story/story-card.tsx`:
- Around line 50-57: The loose type cast on line 52 allows unexpected string
values to pass through for `def?.mode`, which then get used as keys in the
`MODE_DEFAULT_COLOR` and `MODE_LABEL_KEY` lookups on lines 62 and 65. When
assigning to the `mode` variable on line 57, add validation to guard against
invalid `StoryMode` values by checking if `def?.mode` is a valid member of the
`StoryMode` type or enum; if it is not valid, fall back to the default
'creative' value. This ensures only known `StoryMode` values are used in
subsequent object lookups.
In `@components/ui/banner.tsx`:
- Around line 34-43: The Pressable component in the banner.tsx file is removing
the default outline focus indicator with `outline-none` but providing no
replacement visible focus style, making it inaccessible for keyboard navigation.
Either remove the `outline-none` class from the web Platform.select conditional
to restore the default focus indicator, or add a focus-specific style (such as
focus:ring or focus:outline) to provide an alternative visible focus indicator
that keyboard-only users can see when navigating to the CTA.
In `@docs/implementation/triage.md`:
- Around line 22-34: The triage item's description of the story-delete cascade
asset-trashing requirement is incomplete—it currently only mentions the need to
trash orphaned assets from `entry_assets` junctions but does not mention
`stories.cover_asset_id`. Expand the documentation to explicitly state that when
M4/M9 implements refcount-trashing, the hook must cover both `entry_assets`
junction removals and `stories.cover_asset_id` field clearing or story deletion,
so that deleting a story with a cover asset does not leak the blob. Add this
requirement to the triage item's description of what the refcount-trashing slice
must address.
In `@docs/ui/patterns/story-card.md`:
- Around line 46-65: The documentation for `chapterLabel` claims it is a
pre-formatted string computed by the selector (toStoryCardData), but the actual
selector implementation in lib/stores/stories/view-model.ts returns null for
every row. Update the documentation to clarify the current contract for
`chapterLabel` by either marking it as currently deferred or optional (with null
values), or if the implementation will be completed, ensure the documentation
accurately reflects what the selector actually provides at render time.
In `@lib/actions/stories/operational.ts`:
- Around line 20-31: The draft-status guard in the `setStoryArchived` function
is performed outside the transaction before the `ctx.runInTransaction` call,
creating a race condition where concurrent writes can bypass the check. Move the
status validation into the same atomic transaction by adding the draft-status
check as part of the WHERE clause in the update statement, so the guard and the
actual update are enforced atomically together and cannot be separated by
concurrent operations.
In `@lib/stores/stories/stories.ts`:
- Around line 40-43: The getStories function returns direct references to store
properties rows and openFailures, allowing external code to mutate the internal
store state and bypass Zustand's state management. Create defensive copies of
both the rows and openFailures properties when returning from getStories to
ensure that external mutations of the returned object do not affect the
underlying store state.
---
Nitpick comments:
In `@lib/actions/stories/delete-story.ts`:
- Around line 29-45: The comment in the BRANCH_SCOPED declaration states that
assets are excluded as they are handled separately, but entryAssets is actually
included in the array. Update the comment to accurately reflect the current
table list by clarifying which specific assets (if any) are excluded from
BRANCH_SCOPED, or remove entryAssets from the array if it should not be
included. Ensure the comment and the actual list are consistent so future
developers have correct guidance.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d554f30d-e7de-4f9a-b338-a10d2b58d2f0
📒 Files selected for processing (34)
app/dev/story-card.tsxapp/index.tsxcomponents/compounds/toolbar.tsxcomponents/story/app-banner-host.tsxcomponents/story/story-card.stories.tsxcomponents/story/story-card.tsxcomponents/story/story-list.stories.tsxcomponents/story/story-list.tsxcomponents/story/wizard-session-seam.tsxcomponents/ui/banner.stories.tsxcomponents/ui/banner.tsxdocs/data-model.mddocs/implementation/milestones/02-first-user-loop/slices/04-story-list.mddocs/implementation/triage.mddocs/ui/component-inventory.mddocs/ui/patterns/story-card.mdlib/actions/index.tslib/actions/stories/delete-story.test.tslib/actions/stories/delete-story.tslib/actions/stories/operational.test.tslib/actions/stories/operational.tslib/i18n/i18n.test.tslib/stores/__tests__/namespace-shape.test.tslib/stores/index.tslib/stores/stories/relative-time.test.tslib/stores/stories/relative-time.tslib/stores/stories/selectors.test.tslib/stores/stories/selectors.tslib/stores/stories/stories.test.tslib/stores/stories/stories.tslib/stores/stories/view-model.test.tslib/stores/stories/view-model.tslocales/en/common.jsonlocales/en/landing.json
Comment on lines
+20
to
+31
| const [row] = await ctx.db | ||
| .select({ status: stories.status }) | ||
| .from(stories) | ||
| .where(eq(stories.id, id)) | ||
| if (row?.status === 'draft') throw new Error('cannot archive a draft story') | ||
| await ctx.runInTransaction([ | ||
| ctx.db | ||
| .update(stories) | ||
| .set({ status: archived ? 'archived' : 'active' }) | ||
| .where(eq(stories.id, id)) | ||
| .toSQL(), | ||
| ]) |
There was a problem hiding this comment.
Make the draft-archive guard atomic.
setStoryArchived performs the draft-status check before the transactional update, so concurrent writes can invalidate the check and allow an unintended transition. Move the guard into the same atomic write path (or enforce it at DB constraint level) so “cannot archive draft” cannot be bypassed by timing.
🤖 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 `@lib/actions/stories/operational.ts` around lines 20 - 31, The draft-status
guard in the `setStoryArchived` function is performed outside the transaction
before the `ctx.runInTransaction` call, creating a race condition where
concurrent writes can bypass the check. Move the status validation into the same
atomic transaction by adding the draft-status check as part of the WHERE clause
in the update statement, so the guard and the actual update are enforced
atomically together and cannot be separated by concurrent operations.
Comment on lines
+40
to
+43
| function getStories(): StoriesSnapshot { | ||
| const s = store.getState() | ||
| return { rows: s.rows, openFailures: s.openFailures } | ||
| } |
There was a problem hiding this comment.
Return defensive copies from getStories to avoid external mutation of store internals.
Line 42 currently leaks live references (rows, openFailures). Any consumer mutating that object can bypass Zustand updates and silently corrupt shared state.
Proposed fix
function getStories(): StoriesSnapshot {
const s = store.getState()
- return { rows: s.rows, openFailures: s.openFailures }
+ return {
+ rows: [...s.rows],
+ openFailures: { ...s.openFailures },
+ }
}🤖 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 `@lib/stores/stories/stories.ts` around lines 40 - 43, The getStories function
returns direct references to store properties rows and openFailures, allowing
external code to mutate the internal store state and bypass Zustand's state
management. Create defensive copies of both the rows and openFailures properties
when returning from getStories to ensure that external mutations of the returned
object do not affect the underlying store state.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.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
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 by CodeRabbit
Release Notes