Skip to content

fix(td): harden task detail payload and timestamp handling#20

Merged
Helmi merged 1 commit intomainfrom
epic/td-b6a5b7-td-task
Mar 1, 2026
Merged

fix(td): harden task detail payload and timestamp handling#20
Helmi merged 1 commit intomainfrom
epic/td-b6a5b7-td-task

Conversation

@Helmi
Copy link
Owner

@Helmi Helmi commented Mar 1, 2026

Summary

This PR completes td-b6a5b7 (TD Task Detail View Enrichment) and closes its remaining subtask work for timestamp and comments robustness.

What changed

  • Enriched Task Detail modal data handling and rendering:
    • Added robust comments rendering with missing-payload guards.
    • Added scheduling + metadata completeness fields in detail view, including:
      • parent_id
      • creator_session
      • defer_count
  • Hardened TD timestamp parsing and formatting:
    • Correct handling for offset-less td datetimes (UTC normalization).
    • Correct handling for RFC3339 offsets (+HH:MM, +HHMM, Z).
    • Shared timestamp utilities for consistent relative/absolute display.
  • Backend compatibility improvements:
    • Added comments retrieval from td DB.
    • Added compatibility behavior when comments table is missing (returns empty comments, no crash).

Files

  • client/src/components/TaskDetailModal.tsx
  • client/src/components/TaskDetailModal.test.ts
  • client/src/lib/taskDetailLayout.ts
  • client/src/lib/types.ts
  • client/src/lib/tdTimestamp.ts
  • client/src/lib/tdTimestamp.test.ts
  • src/services/tdReader.ts
  • src/services/tdReader.test.ts

Validation

  • bun run test -- client/src/lib/tdTimestamp.test.ts client/src/components/TaskDetailModal.test.ts src/services/tdReader.test.ts src/services/apiServer.test.ts
    • Result: 37/37 tests passed
  • bun run typecheck
    • Passed
  • cd client && bun run build
    • Passed

TD tracking

  • Epic: td-b6a5b7 (closed)
  • Subtasks closed:
    • td-f7106b
    • td-79454a
    • td-cbb80e
    • td-2d8945
    • td-f28e7f

@gemini-code-assist
Copy link

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the Task Detail view by enriching the data displayed and improving its robustness. It introduces a dedicated section for scheduling details, integrates comments directly into the activity feed, and refines how timestamps are parsed and presented. These changes aim to provide a more comprehensive and reliable task overview for users, while also ensuring backend stability with new data structures.

Highlights

  • Enhanced Task Detail Modal: The Task Detail modal now robustly handles and renders comments, and includes new scheduling and metadata fields such as parent_id, creator_session, and defer_count.
  • Improved Timestamp Handling: Timestamp parsing and formatting have been hardened to correctly handle offset-less datetimes (normalizing to UTC) and various RFC3339 offsets. Shared timestamp utilities ensure consistent relative and absolute display across the UI.
  • Backend Comments Integration: The backend now retrieves comments from the td database. Compatibility improvements ensure that the system gracefully handles cases where the comments table might be missing by returning an empty comments array instead of crashing.
Changelog
  • client/src/components/TaskDetailModal.test.ts
    • Imported hasSchedulingDetails utility.
    • Updated makeIssue factory to include new sprint, defer_until, due_date, defer_count, and comments fields.
    • Adjusted points default value in makeIssue to 0.
    • Added new test cases for scheduling details, defer_count handling, acceptance criteria visibility, comments section counting, and graceful handling of missing comments payload.
  • client/src/components/TaskDetailModal.tsx
    • Imported new timestamp formatting utilities (formatTdAbsolute, formatTdDateValue, formatTdRelative, parseTdTimestamp) and hasSchedulingDetails.
    • Replaced the custom parseGoDate function with a new RelativeTime component for consistent timestamp display.
    • Introduced nowMs state and an useEffect hook to update it periodically for accurate relative time rendering.
    • Destructured issue properties (children, handoffs, comments, files, dueDate, deferUntil, sprint, createdBranch) for clearer usage.
    • Added a new 'Comments' collapsible section to the 'Activity' tab, displaying issue comments.
    • Added a new 'Scheduling' collapsible section to the 'Details' tab, showing due_date, defer_until, points, sprint, minor status, defer_count, and created_branch.
    • Updated the 'Metadata' section to use the RelativeTime component for created_at, updated_at, and closed_at timestamps, and added parent_id, creator_session, and defer_count fields.
  • client/src/lib/taskDetailLayout.ts
    • Added hasText and asArray helper functions for data validation and type safety.
    • Introduced hasSchedulingDetails function to determine if an issue has any scheduling-related data.
    • Modified getTaskDetailLayoutCounts to correctly count sections based on the presence of children, handoffs, comments, files, and the new hasSchedulingDetails status.
  • client/src/lib/tdTimestamp.test.ts
    • Added a new test file for tdTimestamp utilities.
    • Included tests for parsing offset-less datetimes as UTC, Go timestamps with offsets, and RFC3339 colon offsets.
    • Added tests for rendering stable relative labels for different timestamp formats.
    • Included a test for formatting date-only values without timezone day shift.
  • client/src/lib/tdTimestamp.ts
    • Added a new utility file for robust timestamp parsing and formatting.
    • Implemented parseOffsetMinutes to handle various offset formats.
    • Provided parseTdTimestamp to normalize different timestamp strings (Go, RFC3339, offset-less) into Date objects, treating offset-less as UTC.
    • Implemented formatTdAbsolute for locale-specific absolute date/time formatting.
    • Implemented formatTdRelative for human-readable relative time display (e.g., '2 hours ago').
    • Implemented formatTdDateValue to format date-only strings, ensuring UTC timezone for consistency.
  • client/src/lib/types.ts
    • Extended TdIssue interface with sprint, defer_until, due_date, and defer_count properties.
    • Defined a new TdComment interface for comment data structure.
    • Added comments array to the TdIssueWithChildren interface.
  • src/services/tdReader.test.ts
    • Added SQL schema for a new comments table in the test database setup.
    • Inserted sample comment data into the test database.
    • Updated the test description for getIssueWithDetails to reflect the inclusion of comments.
    • Added assertions to verify that getIssueWithDetails correctly retrieves comment data.
    • Added a new test case to ensure getIssueWithDetails handles a missing comments table gracefully by returning an empty array.
  • src/services/tdReader.ts
    • Defined the TdComment interface for backend comment data.
    • Extended TdIssueWithChildren interface to include a comments array.
    • Implemented getComments method to query the database for comments associated with an issue, ordered by creation time.
    • Integrated getComments into getIssueWithDetails to fetch comments along with other issue details.
Activity
  • The pull request completes the td-b6a5b7 epic and closes several related subtasks (td-f7106b, td-79454a, td-cbb80e, td-2d8945, td-f28e7f).
  • All client-side and server-side tests (bun run test) passed with 37/37 tests successful.
  • Type checking (bun run typecheck) completed without issues.
  • The client build process (cd client && bun run build) was successful.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@claude
Copy link

claude bot commented Mar 1, 2026

PR Review - fix(td): harden task detail payload and timestamp handling

Overall this is clean, well-structured work. The tdTimestamp module is a solid extraction - the offset normalization logic is correct and the tests pin the key UTC handling contracts. A few things worth addressing before merge.


Bug: defer_count appears twice in the Details tab

defer_count renders in both the Scheduling section (conditionally, when > 0) and the Metadata section (unconditionally, even when 0).

In TaskDetailModal.tsx around line 688, the Scheduling section conditionally renders it. But the Metadata section around line 720 always renders Defer Count / {issue.defer_count} unconditionally.

When defer_count > 0, users see it in two places. When it is 0, it clutters Metadata with a zero that carries no meaning. Either drop it from Metadata (the Scheduling section covers it) or guard it with defer_count > 0 in Metadata.


Unnecessary type cast in taskDetailLayout.ts

Line 62: const comments = asArray((issue as {comments?: unknown}).comments)

TdIssueWithChildren now has comments: TdComment[] as a proper typed field. The cast defeats TypeScript and is no longer needed. It can just be asArray(issue.comments) - the asArray helper still provides the runtime guard for any legacy payloads without the type cast.


hasSchedulingDetails includes created_branch

created_branch triggers the Scheduling section to appear, showing a section titled "Scheduling" containing only a "Branch" row. That is a slightly confusing label for a non-scheduling field. Worth deciding if this is intentional or if created_branch should come back to Metadata.


Minor: nowMs naming

Per the project single-word naming convention, nowMs would typically just be now.


Minor: formatTdAbsolute has no direct test

tdTimestamp.test.ts tests parseTdTimestamp, formatTdRelative, and formatTdDateValue. formatTdAbsolute is exercised indirectly via the title attribute in RelativeTime, but a direct test would complete the module coverage.


What is working well

  • parseTdTimestamp correctly handles all three td timestamp formats (Go monotonic, RFC3339 colon offset, offset-less UTC convention) and the test suite pins each case.
  • The beforeEach/afterEach DB teardown pattern means the DROP TABLE comments test is properly isolated - no cross-test pollution.
  • The try/catch pattern in getComments is consistent with the established file convention (getIssueFiles, getHandoffs, getGitSnapshots all follow the same pattern).
  • The 60s setInterval is appropriate - relative time labels at that granularity do not need sub-minute refresh.
  • Defensive Array.isArray guards on children, handoffs, and files in the modal are a solid robustness improvement.

The defer_count duplication is the only real defect. The cast in taskDetailLayout.ts is a quick cleanup. Rest is solid.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f0c63c0fc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


const offsetToken = tail.match(/^(Z|z|[+-]\d{2}:?\d{2})\b/)?.[1]
const offsetMinutes = offsetToken ? parseOffsetMinutes(offsetToken) : null
const utcMs = Date.UTC(year, month, day, hour, minute, second, ms)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject invalid date parts before constructing UTC timestamp

parseTdTimestamp feeds regex captures directly into Date.UTC, which silently normalizes out-of-range components instead of failing (for example, 2026-02-30 becomes March 2). In environments where td payloads contain malformed datetime/date strings (manual DB edits, partial migrations, or corrupted rows), the modal will display a plausible but wrong time/date rather than falling back to raw text or , which is an accuracy regression from the previous parser behavior.

Useful? React with 👍 / 👎.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request significantly hardens the task detail view by improving timestamp handling and adding robust support for comments and other scheduling fields. The introduction of the tdTimestamp utility is a great improvement for consistency and correctness. The backend changes are also robust, gracefully handling missing database tables. I have a couple of suggestions to address a minor UI redundancy and a style guide violation in the backend.

Comment on lines +720 to +721
<span className="text-muted-foreground">Defer Count</span>
<span>{issue.defer_count}</span>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Defer Count is displayed in the "Metadata" section, but it's also shown in the "Scheduling" section when its value is greater than zero. This creates redundant information in the UI. To avoid this duplication, I suggest removing it from the "Metadata" section, as "Scheduling" is a more appropriate place for this information.

Comment on lines +327 to +339
getComments(issueId: string): TdComment[] {
try {
const db = this.open();
return db
.prepare(
'SELECT * FROM comments WHERE issue_id = ? ORDER BY created_at ASC',
)
.all(issueId) as TdComment[];
} catch (error) {
logger.error(`[TdReader] Failed to get comments for ${issueId}`, error);
return [];
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The new getComments method uses a try/catch block for error handling. While this is consistent with the rest of the file, it deviates from the repository's preferred error handling pattern.

Refactoring this to use Effect-ts would better align with the project's coding standards. This might require a broader refactoring of the TdReader service to be asynchronous and effect-based, which could be considered for future work.

References
  1. The style guide specifies using Effect-ts patterns for error handling instead of raw try/catch blocks. The new getComments function uses a try/catch block. (link)

@Helmi Helmi merged commit 6758faa into main Mar 1, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant