Repository navigation
[3272 by Claude Opus 5.5] Plans relative times parse UTC SQLite timestamps as browser-local time - #3274
Merged
integry merged 1 commit intoOct 19, 2026
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…s UTC. In Europe/London during BST, the issue's two drafts now show 4 minutes ago and 9 minutes ago instead of "1 hour ago". The new tests pass, they fail against the old parsing in both non-UTC zones, and lint and typecheck are clean. The fix (`propr-ui/src/pages/PlansPageUtils.tsx`) - There was no shared parser to reuse. `PlanHistoryDialog.tsx` has its own private one-liner, so I added a small `parseApiTimestamp` in the Plans utils instead. - It only changes how timestamps with no zone are read, like `YYYY-MM-DD HH:mm:ss` with optional fractional seconds: they're now read as UTC. Timestamps that already end in `Z` or a numeric offset like `+01:00` / `-04:00` are passed through unchanged. - `formatRelativeTime` uses the new parser. The thresholds, plurals and the after-30-days `toLocaleDateString()` fallback are unchanged. A missing or invalid value now shows an empty label instead of a "NaN" age. - I didn't change any other timestamp code, the database, or the creation hooks that #3270 touches. Tests (`propr-ui/src/pages/PlansPageUtils.relativeTime.test.tsx`) - Each case runs in Europe/London (BST), America/New_York (UTC−4) and UTC. Each run first checks that the time zone really switched, so the tests can't pass by accident in a UTC-only environment. - They cover the issue's two timestamps, explicit `Z` and `±HH:MM` inputs, and the "just now", minute, hour, day and 30-day boundaries. - One case uses a time near midnight UTC, which falls on a different calendar day in London than in New York, to check the 30-day date fallback. - They also cover a server clock slightly ahead (still "just now") and missing or invalid values. - One case renders the real `PlansList` with a fixed clock and checks it shows "4 minutes ago" with no "hour" text. I also loaded `/plans` in headless Chromium set to Europe/London, at the issue's screenshot time and with the issue's timestamps. The rows read "4 minutes ago" and "9 minutes ago", and I captured that as the preview. The temporary Playwright script I used for this was deleted afterwards.
Owner
|
/review gpt-6-astra |
Contributor
Author
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
Contributor
Author
🔍 AI Code Review — gpt-6-astraOverall EvaluationMerge-ready within the reviewed scope, conditional on pending checks passing. The change narrowly fixes Plans timestamps by normalizing timezone-less date-times to UTC while preserving explicitly zoned timestamps. ✅ Correct UTC normalization — SQLite timestamps become ISO UTC strings before parsing; explicit ✅ Focused regression coverage — Tests cover the reported ages across London, New York, and UTC, plus formatting boundaries, invalid inputs, and rendering through The supplied issue description is truncated; this review covers its visible requirements and the complete supplied diff and context. Validation was static only; no commands or tests were run. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Restore an unset timezone preciselyIn ScoreThe implementation addresses the visible objective with focused coverage and no verified code blockers. Current checks show no failures, with six checks still pending. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
propr-dev Bot
pushed a commit
that referenced
this pull request
Oct 19, 2026
…indow, the changed file can now be scrolled into view with a finger or the mouse wheel, tapped, its diff opened and closed. I tested this in headless Chromium against stubbed task data. It has not been run against real API data or on a real phone. Cause. From 640px wide up, the task page uses the desktop layout. It pins the task header and gives only the area below it its own scroll. In a short window that area was too thin: about 100px in my reproduction, 58px in your measurement. The pinned Timeline and Files headings and the log bar covered nearly all of it, so the file row could never be tapped. Fix. - New `useShortWorkspace.ts`: it measures how much height the header leaves for the timeline/files area. It counts the collapsed log bar and the pinned headings, and allows about 184px for rows. It also respects the visible area when an on-screen keyboard shrinks it, but ignores pinch zoom. The measurement doesn't depend on which layout is active, so it can't flip back and forth. - In `TaskDetails/index.tsx`, when there isn't enough room: - the whole task scrolls as one page, the way phones already do. - the header scrolls away with it, but all its metadata and actions are still there when you scroll back up. - the Timeline and Files headings still pin, now to the top of the page. - the execution-log bar stays pinned at the bottom, and opening it keeps its normal height. - extra scroll padding keeps a focused or revealed row clear of the pinned headings and the log bar. - Unchanged: tall desktop windows, portrait phones, how the timeline heading sits above its content, and returning focus to the file row when the diff closes. The diff dialog's existing scroll lock still holds the page in place. Tests. The new `e2e/task-details-short-viewport.pw.ts` covers 844×390 touch landscape, 880×480 desktop, 320×640 and 390×844 portrait, and 1280×820 desktop. Each uses a completed PR task with a long two-line title and subtitle, 8 runs and one modified 121-line Markdown file. It: - scrolls only with real touch swipes or wheel input. - checks that a tap at the row's centre actually reaches the row and that the visible area is usable. - opens and closes the diff, then checks focus and scroll position are restored. - opens and closes the execution log. - scrolls back up and opens the header's "More task actions" menu. On the old code the two short cases fail the way the issue describes: at 844×390 the row can't be scrolled to a tappable spot, and at 880×480 only 51px of the area is visible. One existing test changed behaviour. In `task-timeline-heading-layering.pw.ts`, the case that docks the task beside the list at 1280×820 with 150% text now uses the new page-scroll layout. There, the enlarged header is 423px tall and leaves only about 97px for rows, the same problem as the issue. The test now scrolls the whole page and still checks that nothing paints over the Timeline heading. Results. All 270 tests in the `task-*` browser suites pass, as do 424 TaskDetails unit tests, eslint with zero warnings and tsc. The new spec passed 3 times in a row. I didn't touch any files from #3273, #3274 or #3275, and added no dependencies. The 844×390 and 880×480 previews show the revealed file row and the opened diff. Separately, Gmail, Google Calendar and Google Drive need authorizing in your claude.ai connector settings before they can be used. This task didn't need them.
integry
pushed a commit
that referenced
this pull request
Oct 20, 2026
…d files covered and untappable (#3277) * I fixed #3276. At 844×390 touch landscape and in an 880×480 desktop window, the changed file can now be scrolled into view with a finger or the mouse wheel, tapped, its diff opened and closed. I tested this in headless Chromium against stubbed task data. It has not been run against real API data or on a real phone. Cause. From 640px wide up, the task page uses the desktop layout. It pins the task header and gives only the area below it its own scroll. In a short window that area was too thin: about 100px in my reproduction, 58px in your measurement. The pinned Timeline and Files headings and the log bar covered nearly all of it, so the file row could never be tapped. Fix. - New `useShortWorkspace.ts`: it measures how much height the header leaves for the timeline/files area. It counts the collapsed log bar and the pinned headings, and allows about 184px for rows. It also respects the visible area when an on-screen keyboard shrinks it, but ignores pinch zoom. The measurement doesn't depend on which layout is active, so it can't flip back and forth. - In `TaskDetails/index.tsx`, when there isn't enough room: - the whole task scrolls as one page, the way phones already do. - the header scrolls away with it, but all its metadata and actions are still there when you scroll back up. - the Timeline and Files headings still pin, now to the top of the page. - the execution-log bar stays pinned at the bottom, and opening it keeps its normal height. - extra scroll padding keeps a focused or revealed row clear of the pinned headings and the log bar. - Unchanged: tall desktop windows, portrait phones, how the timeline heading sits above its content, and returning focus to the file row when the diff closes. The diff dialog's existing scroll lock still holds the page in place. Tests. The new `e2e/task-details-short-viewport.pw.ts` covers 844×390 touch landscape, 880×480 desktop, 320×640 and 390×844 portrait, and 1280×820 desktop. Each uses a completed PR task with a long two-line title and subtitle, 8 runs and one modified 121-line Markdown file. It: - scrolls only with real touch swipes or wheel input. - checks that a tap at the row's centre actually reaches the row and that the visible area is usable. - opens and closes the diff, then checks focus and scroll position are restored. - opens and closes the execution log. - scrolls back up and opens the header's "More task actions" menu. On the old code the two short cases fail the way the issue describes: at 844×390 the row can't be scrolled to a tappable spot, and at 880×480 only 51px of the area is visible. One existing test changed behaviour. In `task-timeline-heading-layering.pw.ts`, the case that docks the task beside the list at 1280×820 with 150% text now uses the new page-scroll layout. There, the enlarged header is 423px tall and leaves only about 97px for rows, the same problem as the issue. The test now scrolls the whole page and still checks that nothing paints over the Timeline heading. Results. All 270 tests in the `task-*` browser suites pass, as do 424 TaskDetails unit tests, eslint with zero warnings and tsc. The new spec passed 3 times in a row. I didn't touch any files from #3273, #3274 or #3275, and added no dependencies. The 844×390 and 880×480 previews show the revealed file row and the opened diff. Separately, Gmail, Google Calendar and Google Drive need authorizing in your claude.ai connector settings before they can be used. This task didn't need them. * feat(ai): Your exact-head failures didn't reproduce in this container, but while ruling out a product cause I found and fixed a real restoration defect in the short layout. That defect plausibly explains the 6px landscape failure. The 1280×820 failure is still unconfirmed: my best explanation is that the wheel scroll hadn't finished when the baseline was measured, and the new wait now covers that. Your exact-head failures didn't reproduce in this container, but while ruling out a product cause I found and fixed a real restoration defect in the short layout. That defect plausibly explains the 6px landscape failure. The 1280×820 failure is still unconfirmed: my best explanation is that the wheel scroll hadn't finished when the baseline was measured, and the new wait now covers that. **Product defect.** In the short layout, the bottom scroll padding was `scroll-pb-12` (48px), but the pinned log bar is 37px (36px plus a 1px top border). When a file row sat fully visible within about 12px above the bar, putting focus back on it as the diff closed counted it as covered and scrolled the page. A diagnostic run placed the row at fixed distances from each edge, then opened and closed the diff: | Row's gap above log bar | 0px | 2px | 5px | 10px | 20px+ | |---|---|---|---|---|---| | Scroll movement, 844×390 and 880×480 | 11px | 9px | 6px | 1px | 0 | The top edge and every position at 1280×820 moved 0px. I set the padding in `useShortWorkspace.ts` to exactly the bar's height (`scroll-pb-[calc(2.25rem+1px)]`). After that change, every position moved 0px in all three sizes. The 1280×820 layout has no such padding, so its 4px isn't explained by this defect. **Test changes** (`e2e/task-details-short-viewport.pw.ts`): - **Waiting for the scroll to end:** the old check was three animation frames without movement. It now waits for the scroll's `scrollend` event and then 300ms with nothing moving. If `scrollend` never fires, it falls back to 5s. - **Baseline position:** it is now measured where the row rests just before it is tapped: after the swipe's momentum, after the preview capture, with a fresh tap point. - **New check in the 880×480 case:** the mouse wheel moves the row to 1–6px above the log bar, then the diff is opened and closed. With the old padding this fails with an 8px difference; with the fix it passes. - **Timing:** I removed a duplicate wait inside the scroll-into-view step and gave these five cases a 60s timeout. Each case does many swipes and each now waits for its momentum to end, so landscape takes about 14s on an idle host and 30–32s under full CPU load, which is over the default 30s. - The 1px limit, the focus and tap-target checks, and real touch/wheel input are unchanged. There are no forced clicks, and the test never sets the scroll position itself. **Results:** - **Repeat runs:** the five viewport cases plus the nine timeline-heading cases passed three times with preview capture and three times without, 84/84. - **Under full CPU load:** the five viewport cases passed once with capture and once without. - **Unit tests, lint and type check:** all 424 TaskDetails unit tests pass, and eslint (zero warnings) and tsc are clean. - **One failure along the way:** before the 60s timeout, an overloaded run timed out in the landscape case and in the unchanged timeline-heading enlarged-text case. **Limits:** - **Old wait never failed here.** It never returned before the scroll ended in this container: not with the CPU slowed, not under load, not with smooth-scrolling Chromium. The 1280×820 explanation therefore comes from your 24-frame result, not from a failure I reproduced. - **Edge diagnostic was not real input.** It set the scroll position directly, and it is not committed. - **Remaining gates:** a real phone, real API data, and signed-native integration are still your release gates. There are no visual previews because the change isn't visible in a still image. Separately, Gmail, Google Calendar and Google Drive need authorizing in your claude.ai connector settings before they can be used; this task didn't need them. PR: #3277 Comment by: @integry (ID: 6279336708) Model: Claude Opus 5.5 * feat(ai): I fixed the Validate Changes failure. The cause was a race in this PR, not the logs dialog: the listener-leak check in `e2e/dialog-keyboard-viewport.pw.ts` measured its baseline before the task page had finished loading. I fixed the Validate Changes failure. The cause was a race in this PR, not the logs dialog: the listener-leak check in `e2e/dialog-keyboard-viewport.pw.ts` measured its baseline before the task page had finished loading. **What went wrong.** The new `useShortWorkspace` hook adds a resize listener on the browser's visual viewport once the task header mounts. The test records the starting (`idle`) listener count right after `page.goto`, before the task data has loaded. So that count is 0, and after the dialog closes the count is 1, the hook's listener for the page itself. The dialogs weren't leaking anything. Locally, this failed every time in the Follow Up test and in both Logs tests (fonts loaded and blocked), not only the one CI reported. **Fix (tests only, no product code changed):** - `e2e/task-logs.fixture.ts`: a new `taskShown(page)` helper waits until the task page has rendered. It reuses the "more actions" button lookup that `openTaskAction` already uses. - `e2e/dialog-keyboard-viewport.pw.ts`: the Follow Up and Logs tests now call `taskShown(page)` before measuring `idle`. The baseline therefore includes the task page's own listeners. The check still fails if a dialog leaves a listener behind. **Verification:** - `dialog-keyboard-viewport`, `task-logs-*` and `task-details-short-viewport` pass, each run twice with retries off (46 passed, 0 failed). Before the fix, `dialog-keyboard-viewport` failed 9 of 15 runs. - ESLint (`--max-warnings 0`) and `tsc --noEmit` pass on the changed files. This change isn't visible in the UI, so there are no previews. PR: #3277 Comment by: @propr-dev[bot] (ID: 6280778125) Comment by: @github-actions[bot] (ID: 6280778384) Model: Claude Opus 5.5 --------- Co-authored-by: propr-dev[bot] <1316198+propr-dev[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.
AI Implementation Summary
Closes #3272
Branch:
3272/claude-opus-5-5-plans-relative-times-pars-20261019-2154-ojfCommits: ✅ Changes committed (ec238ea)
AI Processing Completed
Execution Details:
Summary:
The Plans list now treats the API's timezone-less SQLite timestamps as UTC. In Europe/London during BST, the issue's two drafts now show 4 minutes ago and 9 minutes ago instead of "1 hour ago". The new tests pass, they fail against the old parsing in both non-UTC zones, and lint and typecheck are clean.
The fix (
propr-ui/src/pages/PlansPageUtils.tsx)PlanHistoryDialog.tsxhas its own private one-liner, so I added a smallparseApiTimestampin the Plans utils instead.YYYY-MM-DD HH:mm:sswith optional fractional seconds: they're now read as UTC. Timestamps that already end inZor a numeric offset like+01:00/-04:00are passed through unchanged.formatRelativeTimeuses the new parser. The thresholds, plurals and the after-30-daystoLocaleDateString()fallback are unchanged. A missing or invalid value now shows an empty label instead of a "NaN" age.Tests (
propr-ui/src/pages/PlansPageUtils.relativeTime.test.tsx)Zand±HH:MMinputs, and the "just now", minute, hour, day and 30-day boundaries.PlansListwith a fixed clock and checks it shows "4 minutes ago" with no "hour" text.I also loaded
/plansin headless Chromium set to Europe/London, at the issue's screenshot time and with the issue's timestamps. The rows read "4 minutes ago" and "9 minutes ago", and I captured that as the preview. The temporary Playwright script I used for this was deleted afterwards.Detailed Logs:
e0f984f5-7ee7-4051-8e92-4e636b53b7c0Log files stored at:
/usr/src/app/logs/executions/issue-3272-2026-10-19T22-00-01-691Z-conversation.json/usr/src/app/logs/executions/issue-3272-2026-10-19T22-00-01-691Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #3272.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!
Visual preview
Plans list in Europe/London (BST)
Plans list at 2026-10-19T21:44:43Z with the API returning SQLite UTC timestamps 21:40:37 and 21:35:43. The rows now read 4 and 9 minutes ago instead of 1 hour ago.
Original unavailable: managed storage is unavailable or the upload has expired.