fix: stabilize session navigation ordering by Astro-Han · Pull Request #367 · Astro-Han/pawwork

Astro-Han · 2026-05-02T03:12:43Z

Summary

Prevent the session review file derivation from crashing while turn diffs are still loading.
Sort PawWork sidebar sessions by the latest loaded user message timestamp, with a creation-time fallback when messages are missing or invalid.
Reject non-finite timestamp values before they can reach sidebar sorting.
Add focused regression coverage for missing turn diffs, missing message caches, assistant-only caches, malformed user timestamps, and non-finite timestamps.

Why

A newer build could throw Cannot read properties of undefined (reading 'flatMap') when opening a session before turnDiffs arrived. The sidebar also used session.time.updated as a fallback, which let assistant/background updates promote older conversations and made timestamps/order feel wrong. The helper keeps the ordering tied to user activity when available and avoids using update time as a fallback. Review also identified that NaN or Infinity timestamps could still reach the numeric sort key, so the helper now accepts only finite numbers.

Related Issue

No linked issue. This came from local regression triage against real session logs.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

Please focus on the fallback ordering contract in pawworkSidebarSessionTime: use the latest valid loaded user message time, otherwise fall back to session.time.created, never session.time.updated, and never return non-finite numbers. Also check that treating missing turnDiffs as an empty list does not hide legitimate review files once data arrives.

Risk Notes

Low. This changes sidebar ordering behavior and a loading-state fallback only. No persistence, dependency, migration, permissions, updater, or generated-file changes are included.

How To Verify

TDD red check: malformed user timestamp regression failed before the helper fix with 1 failing test.
Review TDD red check: non-finite timestamp regressions failed before the finite-number guard with 2 failing tests.
Focused sorting test: bun --cwd packages/app test --preload ./happydom.ts ./src/pages/layout/pawwork-session-source.test.ts -> 656 pass, 0 fail
Focused regression tests: bun --cwd packages/app test --preload ./happydom.ts ./src/pages/layout/pawwork-session-source.test.ts ./src/pages/session/use-session-review-state.test.ts -> 656 pass, 0 fail
Typecheck: bun run --cwd packages/app typecheck -> tsgo -b completed successfully
Diff check: git diff --check -> no output
Manual dev check: ran dev against a copied production PawWork DB and clicked real sessions without reproducing the crash; verified sessions without loaded messages fall back to creation time rather than updated time.
Scope check: PR diff contains only app source/test files; unrelated models snapshot change and .pawwork-verify/ were left out.

Screenshots or Recordings

Not captured. This is a behavior and loading-state fix with no visual layout or copy change.

Checklist

coderabbitai · 2026-05-02T03:13:03Z

📝 Walkthrough

Walkthrough

Session timestamp calculation logic is extracted from inline implementation in layout.tsx to a new pawworkSidebarSessionTime helper that selects the latest user message's creation time, falling back to session creation time. A separate change makes turnDiffs optional in deriveReviewArtifactFiles to handle undefined inputs gracefully.

Changes

Session Timestamp Extraction

Layer / File(s)	Summary
Type Definitions & Helper `packages/app/src/pages/layout/pawwork-session-source.ts`	Added `SessionTimeLike` and `MessageTimeLike` types. Exported `pawworkSidebarSessionTime(session, messages?)` helper that iterates messages from newest to oldest, selecting the first `"user"` message with a finite `created` timestamp, falling back to `session.time?.created` or `0`.
Usage Integration `packages/app/src/pages/layout.tsx`	Updated import to include `pawworkSidebarSessionTime`. Replaced inline reverse-scan logic in `collectPawworkSessions` with a call to the new helper function.
Tests `packages/app/src/pages/layout/pawwork-session-source.test.ts`	Added comprehensive test coverage for `pawworkSidebarSessionTime`: latest user message selection, invalid/missing timestamp handling, fallback to session creation time, and return of `0` when all timestamps are unavailable. Expanded `sortPawworkSidebarSessions` tests for message-based ordering and assistant-only-message filtering.

Optional turnDiffs Parameter

Layer / File(s)	Summary
Function Signature `packages/app/src/pages/session/use-session-review-state.ts`	Made `turnDiffs` parameter optional in `deriveReviewArtifactFiles` input. Added inline default resolution `turnDiffs ?? []` to prevent undefined access in downstream logic.
Tests `packages/app/src/pages/session/use-session-review-state.test.ts`	Added test cases verifying function returns empty array when `turnDiffs` is undefined for a loading session, and that function does not throw when turn diffs have not arrived.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

fix: order sessions by creation time #115: Switches pawwork session ordering to use creation time rather than update time, directly related to the new pawworkSidebarSessionTime helper that prioritizes user message timing for session sorting.

Suggested labels

P1

🐰 New helpers hop into place,
Sessions trace their message race,
Timestamps flow from user's pen,
Optional diffs feel right again! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: stabilize session navigation ordering' accurately summarizes the main change: stabilizing/fixing how sessions are ordered in navigation by switching to user message timestamps instead of update time.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description comprehensively addresses all required template sections with detailed explanations of changes, reasoning, risks, and verification steps.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch codex/fix-session-click-time

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

gemini-code-assist

Code Review

This pull request refactors the session time calculation for the sidebar by introducing a new helper function, pawworkSidebarSessionTime, which prioritizes the timestamp of the latest user message over the session's creation time. This change is supported by new unit tests and integrated into the layout logic. Additionally, the PR updates deriveReviewArtifactFiles to safely handle undefined turnDiffs during session loading. The review feedback suggests improving the robustness of timestamp validation by using Number.isFinite instead of a simple type check to avoid potential issues with NaN or Infinity values.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/app/src/pages/layout/pawwork-session-source.test.ts (1)

152-185: ⚡ Quick win

Add a regression case for NaN user/session timestamps.

These tests are solid; one more case for non-finite numbers would prevent ordering regressions from malformed numeric timestamps.

Suggested test addition

 describe("pawworkSidebarSessionTime", () => {
+  test("falls back when user/session created timestamps are NaN", () => {
+    expect(
+      pawworkSidebarSessionTime(
+        { time: { created: Number.NaN } },
+        [{ id: "msg_nan", role: "user", time: { created: Number.NaN } }],
+      ),
+    ).toBe(0)
+  })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/app/src/pages/layout/pawwork-session-source.test.ts` around lines
152 - 185, Add a regression test to pawwork-session-source.test that ensures
pawworkSidebarSessionTime correctly handles non-finite numeric timestamps (e.g.,
NaN) by treating them as missing and falling back to the next valid source;
specifically add a case where a session or user message has time.created: NaN
(or Number.NaN) and assert the function returns the valid alternate timestamp
(or 0 if none), referencing the pawworkSidebarSessionTime function and existing
tests "ignores user messages without a valid created time" / "uses the session
creation time..." as templates.

🤖 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/pages/layout/pawwork-session-source.ts`:
- Around line 64-72: In pawworkSidebarSessionTime, the current check uses typeof
created === "number" which allows NaN/Infinity; update the validation to use
Number.isFinite(created) when returning a message-created timestamp and also use
Number.isFinite(session.time?.created) for the session fallback so sorting
(e.g., b.created - a.created) never receives non-finite values; reference the
created local variable and session.time?.created in your changes to locate the
logic.

---

Nitpick comments:
In `@packages/app/src/pages/layout/pawwork-session-source.test.ts`:
- Around line 152-185: Add a regression test to pawwork-session-source.test that
ensures pawworkSidebarSessionTime correctly handles non-finite numeric
timestamps (e.g., NaN) by treating them as missing and falling back to the next
valid source; specifically add a case where a session or user message has
time.created: NaN (or Number.NaN) and assert the function returns the valid
alternate timestamp (or 0 if none), referencing the pawworkSidebarSessionTime
function and existing tests "ignores user messages without a valid created time"
/ "uses the session creation time..." as templates.

🪄 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: b44cb2cd-c8b8-44dc-9bab-a2e60a859351

📥 Commits

Reviewing files that changed from the base of the PR and between 3816c66 and 0774f02.

📒 Files selected for processing (5)

packages/app/src/pages/layout.tsx
packages/app/src/pages/layout/pawwork-session-source.test.ts
packages/app/src/pages/layout/pawwork-session-source.ts
packages/app/src/pages/session/use-session-review-state.test.ts
packages/app/src/pages/session/use-session-review-state.ts

Root cause: - PawWork's sidebar row builder in `packages/app/src/pages/layout.tsx` still populated `created` from `session.time.created`. - `PawworkSidebar` used that same field for the relative time text, and `buildPawworkSessionSections` used it for time sorting. - PR #367 had already established the correct loaded-row contract in `pawworkSidebarSessionTime`: latest valid loaded user message time, otherwise session creation time. The PawWork session-window path simply had not been wired to that helper. Behavior change: - Sidebar rows that already have loaded message cache now use the latest valid user message `time.created` for both displayed relative time and row-level time sorting. - Rows with missing cache, assistant-only messages, invalid user timestamps, or no user messages still fall back to `session.time.created`. - The cache read uses `globalSync.child(directory, { bootstrap: false, pin: false })`, so it does not bootstrap or owner-pin child stores just to calculate sidebar timestamps. Scope boundary: - No backend API, database schema, persistence, desktop shell, packaging, permission, model harness, or visual layout changes. - The initial global session window is still selected by the existing created-time API window. The broader product/API change to fetch the first sidebar window by latest user message time is tracked separately in #473. - A lower-priority cache API follow-up for reading an existing child store without creating one is tracked in #475. - Broader layout.tsx controller extraction is tracked in #474 and intentionally kept out of this bug fix. Tests and verification: - Added `buildPawworkSidebarSessionRows` coverage for loaded user-message timestamps. - Added missing-message-cache fallback coverage for the row builder. - Existing helper tests continue to cover assistant-only caches and invalid timestamp fallback. - TDD red check: the focused app unit test failed before implementation because `buildPawworkSidebarSessionRows` did not exist. - Focused app tests: `bun --cwd packages/app test --preload ./happydom.ts ./src/pages/layout/pawwork-session-source.test.ts` -> 916 pass, 0 fail. - App typecheck: `bun --cwd packages/app typecheck` passed. - Workspace typecheck: `bun turbo typecheck` passed before review follow-up. - Diff check: `git diff --check` passed. - Desktop startup check: `bun run dev:desktop` built main/preload/renderer and started Electron, but an existing local PawWork dev instance using the shared dev profile blocked a full interactive sidebar click-through in this worktree. - PR checks were green before merge, including typecheck, unit-app, unit-desktop, unit-opencode, e2e-artifacts, CodeQL, and smoke-macos-arm64. Closes #470

fix: stabilize session navigation ordering

0774f02

Astro-Han added bug Something isn't working app Application behavior and product flows P0 Blocking / highest priority labels May 2, 2026

Astro-Han marked this pull request as ready for review May 2, 2026 03:14

gemini-code-assist Bot reviewed May 2, 2026

View reviewed changes

Comment thread packages/app/src/pages/layout/pawwork-session-source.ts Outdated

coderabbitai Bot reviewed May 2, 2026

View reviewed changes

Comment thread packages/app/src/pages/layout/pawwork-session-source.ts

fix: reject non-finite session timestamps

2be5524

Astro-Han merged commit 1ece9f9 into dev May 2, 2026
27 of 28 checks passed

coderabbitai Bot mentioned this pull request May 2, 2026

fix(app): stabilize sidebar session history #386

Merged

11 tasks

Astro-Han deleted the codex/fix-session-click-time branch May 2, 2026 14:12

coderabbitai Bot mentioned this pull request May 6, 2026

feat(app): sort sidebar sessions by user activity #476

Merged

11 tasks

coderabbitai Bot mentioned this pull request May 14, 2026

fix(app): stop auto-opening files panel #613

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: stabilize session navigation ordering#367

fix: stabilize session navigation ordering#367
Astro-Han merged 2 commits into
devfrom
codex/fix-session-click-time

Astro-Han commented May 2, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 2, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Astro-Han commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Related Issue

Human Review Status

Review Focus

Risk Notes

How To Verify

Screenshots or Recordings

Checklist

Uh oh!

coderabbitai Bot commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Astro-Han commented May 2, 2026 •

edited

Loading

coderabbitai Bot commented May 2, 2026 •

edited

Loading