Skip to content

Collapse hidden GitHub comments in detail views - #818

Open
mariusvniekerk wants to merge 13 commits into
mainfrom
t3code/show-hidden-comments-collapsed
Open

Collapse hidden GitHub comments in detail views#818
mariusvniekerk wants to merge 13 commits into
mainfrom
t3code/show-hidden-comments-collapsed

Conversation

@mariusvniekerk

@mariusvniekerk mariusvniekerk commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

GitHub-hidden comments previously appeared like ordinary discussion content in PR and issue details.

  • Preserve GitHub’s hidden state and reason through sync refreshes.
  • Default hidden comments to a collapsed closed-eye notice without previewing their text.
  • Keep the original content available through an explicit Show comment action.

Hidden GitHub comment collapsed by default

generated by a clanker

@mariusvniekerk

Copy link
Copy Markdown
Collaborator Author

Hidden GitHub comment collapsed by default

generated by a clanker

@roborev-ci

roborev-ci Bot commented Aug 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (92c6c02)

The change is directionally sound, but three medium-severity gaps leave minimized comments exposed in some paths.

Medium

  • Incomplete pagination for visibility metadatainternal/github/graphql.go:76,334
    Visibility is queried only for the first 100 comments. If the connection is truncated, later comments supplied by REST lack minimization fields and render normally. Paginate the GraphQL comment connection and merge the complete ID-to-visibility map with REST comments before persistence.

  • Inline review comments lack moderation stateinternal/github/graphql.go:671
    Only pull-request conversation comments are handled. Inline review comments use the separate review-thread GraphQL path, which does not request or persist isMinimized/minimizedReason; hidden review comments and threaded replies remain fully visible. Propagate moderation state through the review-thread models and MetadataJSON, including reply rendering.

  • End-to-end data flow is untestedinternal/github/graphql_test.go:77
    Tests construct GraphQL structs and UI events directly, so they do not verify the GraphQL-to-SQLite-to-HTTP path. Add a real HTTP GraphQL fixture and API/SQLite integration test proving minimization metadata reaches the detail response, including paginated comments.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 7m5s

@mariusvniekerk mariusvniekerk self-assigned this Aug 3, 2026
@roborev-ci

roborev-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (e779c97)

One medium-severity issue remains in GitHub comment moderation handling.

Medium

  • internal/github/graphql.go:76, internal/github/graphql.go:334 — Moderation state is fetched only for the first 100 comments. Since the REST fallback supplies comment bodies but not GraphQL-only minimized state, previously unseen comments beyond that page remain visible instead of collapsed. Paginate the nested comments connection and merge visibility metadata for every comment into the complete REST list. Add coverage for a newly minimized comment beyond the first page.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 5m21s

@mariusvniekerk
mariusvniekerk force-pushed the t3code/show-hidden-comments-collapsed branch from e779c97 to 1966f34 Compare August 5, 2026 17:28
@roborev-ci

roborev-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (1966f34)

Medium-severity issues remain in moderation-state synchronization and PR coverage.

Medium

  • internal/github/graphql.go:713, internal/github/graphql.go:1105 — Visibility is collected only from comments(first: 100). During an initially incomplete sync, later REST comments lack stored moderation state, causing minimized comments beyond the first page—including unseen closed-item comments—to appear visible. Paginate or separately query GraphQL moderation state for every REST-returned comment before publishing the completed snapshot.

  • internal/github/sync.go:2201, internal/github/sync.go:2266 — Editing a hidden comment returns a REST-normalized event without moderation metadata, while the upsert unconditionally replaces metadata_json. This removes provider_hidden and exposes comments that may still be minimized on GitHub. Preserve stored moderation metadata across edit responses until GraphQL explicitly confirms visibility.

  • internal/server/api_test.go:12656 — Full-stack coverage tests only issue comments, although pull requests use a different partial-sync persistence path. Add a PR end-to-end test covering initial hidden-state persistence and preservation through an incomplete GraphQL page followed by REST completion.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 4m50s

@roborev-ci

roborev-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (60b1a6f)

Changes need revisions: three medium-severity gaps remain in hidden-comment synchronization, rendering, and end-to-end coverage.

Medium

  • Hidden thread replies are exposed by defaultpackages/ui/src/components/detail/EventTimeline.svelte:2117
    Thread replies bypass the provider-hidden check and render their bodies directly. Since threads start expanded, a minimized inline-review reply is exposed even though standalone and root hidden comments are collapsed. Apply providerHiddenState and the notice/reveal gating to every reply, and test a visible root containing a hidden reply.

  • Detail refreshes can miss current moderation stateinternal/github/sync.go:9181, internal/github/sync.go:10066
    REST detail refreshes reuse stored moderation state but do not query current GraphQL visibility. Hidden issue comments may therefore display normally for closed items, large repositories where bulk GraphQL is skipped, or syncs performed while GraphQL is unavailable. Fetch per-item comment visibility during GitHub PR and issue detail refreshes, using stored state only when the GraphQL observation is unavailable.

  • The collapse/reveal workflow lacks browser-level coveragepackages/ui/src/components/detail/EventTimeline.svelte:1535
    Component tests do not verify that moderation metadata from the real detail API reaches the timeline and remains collapsed until explicitly revealed. Add Playwright coverage using a seeded hidden PR or issue comment, asserting both the default collapsed state and reveal action.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 6m33s

@roborev-ci

roborev-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (9122913)

Moderation metadata support is incomplete, leaving hidden review comments exposed or losing their hidden state.

Medium

  • internal/github/sync.go:2662 — Review-thread GraphQL queries and DTO conversion do not request or decode isMinimized and minimizedReason. Hidden inline comments therefore cannot receive moderation metadata through the real provider path.

    • Fix: Add both fields to the review-thread DTOs, initial and paginated GraphQL selections, and conversion logic, with provider-client pagination coverage.
  • packages/ui/src/components/diff/DiffReviewThreadInlineComment.svelte:146 — The timeline collapses hidden comments, but the Diff view renders thread.body unconditionally. The diff-thread API response also omits moderation metadata, exposing minimized content elsewhere in the PR detail view.

    • Fix: Propagate the metadata through diffReviewThreadResponse, regenerate API artifacts, and render a collapsed notice/toggle with coverage.
  • internal/server/pullapi/diff_review_handlers.go:843ingestDiffReviewThreads copies metadata to thread rows but not corresponding review_comment events. Publishing a review re-ingests threads and upserts empty metadata_json, clearing stored hidden markers.

    • Fix: Copy thread.MetadataJSON into each event or reuse platform.DBReviewThreads, and add a full-stack review-publish regression test.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 8m2s

@roborev-ci

roborev-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown

roborev: Combined Review (f18c3e9)

High-severity correctness issue: conditional refreshes can leave GitHub comment moderation state stale; two additional medium-severity reliability concerns remain.

High

  • Stale comment visibility after conditional refreshesinternal/github/sync.go:8380, internal/github/sync.go:8917, internal/github/sync.go:10968
    A parent PR or issue returning 304 Not Modified causes the detail refresh to exit before querying the new GraphQL-only comment visibility. Because moderation changes may not alter the parent REST representation, hidden comments can remain exposed and restored comments can remain collapsed across repeated refreshes.
    Fix: On 304, still fetch and apply comment visibility to stored comment events, or track moderation freshness separately. Add regression tests for both hidden and restored states following a parent 304.

Medium

  • Visibility queries bypass GraphQL quota admissioninternal/github/sync.go:9185, internal/github/sync.go:10087
    The new queries run whenever a fetcher exists, without applying the credential-aware GraphQL rate-limit, backoff, and reserve policy used elsewhere. Background detail drains may repeatedly attempt exhausted quota for every PR and issue.
    Fix: Gate these optional queries through the existing GraphQL admission policy and preserve stored metadata when admission is denied.

  • Thread and timeline moderation state can divergeinternal/server/pullapi/diff_review_handlers.go:856
    Review-thread moderation metadata is persisted to thread rows and timeline events in separate transactions. A failure after the thread upsert can leave a hidden comment collapsed in the diff but exposed in the timeline, potentially durably when ingestion errors are discarded.
    Fix: Delete missing rows and upsert thread/event representations atomically in one transaction, and propagate or retry ingestion failures.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 11m44s

@roborev-ci

roborev-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown

roborev: Combined Review (c5b5e42)

Review verdict: Changes need revision due to one high-severity publish-state bug and three medium-severity synchronization/data-integrity issues.

High

  • internal/server/pullapi/diff_review_handlers.go:606 — After a provider successfully publishes a review and the local draft is deleted, a transient thread-read or persistence failure returns an HTTP error. Because publishing is irreversible, this false failure may cause users to recreate or duplicate the review.
    • Fix: Make post-publish ingestion best-effort: log or schedule a refresh and return published. Likewise, preserve a partial-publish response instead of replacing it with an ingestion error.

Medium

  • internal/github/sync.go:8579 — The 304 Not Modified path refreshes only ordinary PullRequest.comments and updates only issue_comment rows. Inline review_comment visibility and review-thread rows can remain stale when the parent PR is unchanged, leaving newly minimized comments expanded.

    • Fix: Under the same revision guard, refresh review-thread visibility when GraphQL admission permits and atomically update thread and timeline-event metadata. Add conditional-sync end-to-end coverage.
  • internal/github/graphql.go:154 — Visibility records use GitHub’s legacy databaseId, but issue-comment IDs can exceed its range. Such comments may map to zero or a mismatched ID, causing hidden state to be dropped when matched with REST IDs.

    • Fix: Query and decode fullDatabaseId for gqlComment and gqlCommentVisibilityNode, preferring it over databaseId, and test IDs above the 32-bit range.
  • internal/server/pullapi/routes.go:1002, internal/server/issueapi/mutation_handlers.go:92 — Edit handlers read moderation metadata and upsert the edited event separately. A concurrent visibility refresh can be overwritten with stale metadata, exposing a newly hidden comment or incorrectly restoring a hidden marker.

    • Fix: Atomically persist the edited body while preserving current provider visibility metadata in the database.

Reviewers: 2 done | Synthesis: codex, 15s | Total: 11m37s

@roborev-ci

This comment has been minimized.

mariusvniekerk added a commit that referenced this pull request Aug 6, 2026
A parent PR ETag does not cover inline review-comment moderation, so conditional detail refreshes must reconcile review threads under the same quota and snapshot rules as conversation comments.

Provider publication is irreversible. Follow-up thread ingestion now reconciles asynchronously on failure without replacing a published or partially-published result and inviting duplicate reviews.

Triage decision for roborev-ci #818 (comment): user: not an issue — GitHub is authoritative, and the non-atomic edit/visibility edge case is not worth additional complexity.

Validation: focused moderation/publish API tests; full internal/github and internal/server/pullapi suites; golangci-lint. The full internal/server suite reached an unrelated quick-exit timing flake, which passed when rerun alone.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown

roborev: Combined Review (fda0e13)

Code review passed: no Medium, High, or Critical findings.


Reviewers: 2 done | Synthesis: codex, 7s | Total: 9m28s

mariusvniekerk added a commit that referenced this pull request Aug 6, 2026
A parent PR ETag does not cover inline review-comment moderation, so conditional detail refreshes must reconcile review threads under the same quota and snapshot rules as conversation comments.

Provider publication is irreversible. Follow-up thread ingestion now reconciles asynchronously on failure without replacing a published or partially-published result and inviting duplicate reviews.

Triage decision for roborev-ci #818 (comment): user: not an issue — GitHub is authoritative, and the non-atomic edit/visibility edge case is not worth additional complexity.

Validation: focused moderation/publish API tests; full internal/github and internal/server/pullapi suites; golangci-lint. The full internal/server suite reached an unrelated quick-exit timing flake, which passed when rerun alone.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk
mariusvniekerk force-pushed the t3code/show-hidden-comments-collapsed branch from fda0e13 to c1e89be Compare August 6, 2026 16:46
@roborev-ci

roborev-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown

roborev: Combined Review (c1e89be)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 10m8s

@roborev-ci

This comment has been minimized.

@roborev-ci

This comment has been minimized.

mariusvniekerk added a commit that referenced this pull request Aug 7, 2026
Bulk sync could publish a complete conversation-comment snapshot without current inline review threads, then mark the pull request detail as fresh. That left hidden review replies stale until another detail refresh.

Load the initial conversation-comment and review-thread connections in one GitHub GraphQL snapshot. Keep later conversation pages on GraphQL so comment content and moderation share one authority, and leave detail stale whenever either discussion family is incomplete.

GitHub's live API accepted the production page-size query and returned both connections for PR #830. This verifies the schema and query-complexity limit beyond local stubs.

Triage finding fixed: #818 (comment)

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (09b96a6)

Review identifies one high-severity coverage gap and one medium-severity activity-ordering bug.

High

  • internal/server/api_test.go:13510 — The combined GraphQL bulk review-thread path lacks end-to-end coverage. The new PR fixture omits reviewThreads, so hidden-thread ingestion could regress while tests remain green. Include hidden review threads in a bulk GraphQL response and assert their event/thread metadata through the real API and SQLite.

Medium

  • internal/github/sync.go:8369, internal/github/sync.go:9079 — Review-thread events do not contribute to last_activity_at. A newer inline reply may appear while dashboard ordering and refresh scheduling retain an older timestamp. Include review-thread creation and update times when atomically computing derived merge-request activity.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 10m48s

mariusvniekerk added a commit that referenced this pull request Aug 7, 2026
A parent PR ETag does not cover inline review-comment moderation, so conditional detail refreshes must reconcile review threads under the same quota and snapshot rules as conversation comments.

Provider publication is irreversible. Follow-up thread ingestion now reconciles asynchronously on failure without replacing a published or partially-published result and inviting duplicate reviews.

Triage decision for roborev-ci #818 (comment): user: not an issue — GitHub is authoritative, and the non-atomic edit/visibility edge case is not worth additional complexity.

Validation: focused moderation/publish API tests; full internal/github and internal/server/pullapi suites; golangci-lint. The full internal/server suite reached an unrelated quick-exit timing flake, which passed when rerun alone.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk
mariusvniekerk force-pushed the t3code/show-hidden-comments-collapsed branch from 09b96a6 to 42c3e86 Compare August 7, 2026 14:22
mariusvniekerk added a commit that referenced this pull request Aug 7, 2026
Bulk sync could publish a complete conversation-comment snapshot without current inline review threads, then mark the pull request detail as fresh. That left hidden review replies stale until another detail refresh.

Load the initial conversation-comment and review-thread connections in one GitHub GraphQL snapshot. Keep later conversation pages on GraphQL so comment content and moderation share one authority, and leave detail stale whenever either discussion family is incomplete.

GitHub's live API accepted the production page-size query and returned both connections for PR #830. This verifies the schema and query-complexity limit beyond local stubs.

Triage finding fixed: #818 (comment)

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
mariusvniekerk added a commit that referenced this pull request Aug 7, 2026
Inline review replies were synchronized without contributing their provider timestamps to last_activity_at. A newer reply could therefore leave dashboard ordering and warm refresh scheduling anchored to older activity.

Use GitHub's GraphQL creation and update times in both bulk and detail-thread snapshots, without inventing local timestamps. The combined GraphQL fixture now crosses synchronization, SQLite, and the pull-detail API, including incomplete-thread preservation.

Live GitHub validation confirmed that both review-thread query forms return the timestamps. This fixes local roborev job 10346 and #818 (comment).

Validation: full internal/github and internal/server packages; focused combined GraphQL/API regressions; gated live GitHub GraphQL test; golangci-lint.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (42c3e86)

Review verdict: Three medium-severity issues require attention; no high or critical findings were identified.

Medium

  • internal/github/sync.go:9474 — Review-thread-only refreshes retain the prior LastActivityAt. Deleting the newest inline comment removes its event but leaves its timestamp, making derived activity inconsistent with persisted events. Recompute activity from remaining datasets atomically with thread replacement, then include incoming thread timestamps.

  • internal/server/pullapi/diff_review_handlers.go:869 — Successful review publication persists new thread events without advancing last_activity_at, and no reconciliation sync follows successful ingestion. Update revision-guarded derived fields using the newest ingested thread timestamp.

  • packages/ui/src/components/detail/EventTimeline.svelte:2048 — The collapsed hidden-comment workflow lacks Playwright e2e coverage through the real HTTP API and SQLite. Add an e2e test confirming hidden timeline and diff comments initially render collapsed and expand on request.


Reviewers: 2 done | Synthesis: codex, 6s | Total: 5m12s

mariusvniekerk and others added 5 commits August 9, 2026 12:27
GitHub-hidden discussion comments were previously indistinguishable from ordinary comments after sync, so PR and issue detail views could expose text the provider intentionally minimized. Preserve the GraphQL moderation state across the REST-shaped sync pipeline and keep REST refreshes from discarding it until an authoritative GraphQL snapshot changes the state.\n\nDefault hidden comments to a compact closed-eye notice with the provider reason while retaining an explicit path to inspect the original content. The hook-enforced frontend check also normalizes two pre-existing line wraps required by the current formatter.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
Incomplete GraphQL comment pages only describe the comments they observed. Treating that partial visibility map as authoritative during REST completion could silently expose previously minimized comments outside the page.\n\nRecord visible observations explicitly and merge partial moderation state over the stored snapshot so GitHub can still clear stale hidden metadata without discarding state it did not return.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
GitHub's GraphQL comment pages can be incomplete, while REST detail and edit responses omit the provider's minimized state. Without carrying the complete moderation map through REST replacement and mutation persistence, hidden comments could reappear in PR and issue details.\n\nHydrate later GraphQL comment pages before REST completion, preserve confirmed hidden metadata during edits, and cover both synchronization paths through provider and HTTP-level regression tests.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
REST detail refreshes can return comment bodies without GitHub's minimized metadata, so stored state alone can expose comments that were hidden or fail to notice newly minimized comments. Re-observe per-item moderation state through GraphQL when available and preserve stored state only when that observation is unavailable.\n\nApply the same provider-hidden notice and reveal gate to threaded replies so an expanded review thread cannot bypass the root comment's privacy treatment. Add GraphQL, REST detail, and visible-root/hidden-reply regression coverage.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
GitHub review-thread GraphQL omitted moderation fields, while the diff-thread API and review-publish re-ingestion dropped metadata even after sync observed it. That allowed minimized inline comments to appear in diff cards or become visible again after publishing a review.

Carry moderation state through initial and paginated provider reads, both persisted thread representations, and the generated API contract. Diff cards now default hidden comments to a collapsed notice with an explicit reveal action.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
mariusvniekerk and others added 8 commits August 9, 2026 12:31
Parent ETags do not cover comment moderation changes, so conditional detail refreshes now re-observe visibility when the routed credential has GraphQL capacity and apply observations under the parent revision guard.

Use the shared GraphQL admission policy for optional visibility reads, and commit review-thread rows with their timeline events atomically so the two privacy representations cannot diverge.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
The moderation pagination changes used a manual map copy and fatal test assertions inside HTTP handler goroutines, which the repository lint policy rejects. Keep the same test behavior while making handler failures non-fatal and using the standard map-copy primitive so the normal CI gate can complete.

Validation: golangci-lint run; focused GraphQL visibility tests.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
A parent PR ETag does not cover inline review-comment moderation, so conditional detail refreshes must reconcile review threads under the same quota and snapshot rules as conversation comments.

Provider publication is irreversible. Follow-up thread ingestion now reconciles asynchronously on failure without replacing a published or partially-published result and inviting duplicate reviews.

Triage decision for roborev-ci #818 (comment): user: not an issue — GitHub is authoritative, and the non-atomic edit/visibility edge case is not worth additional complexity.

Validation: focused moderation/publish API tests; full internal/github and internal/server/pullapi suites; golangci-lint. The full internal/server suite reached an unrelated quick-exit timing flake, which passed when rerun alone.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
A published response alone did not prove that the background sync repaired a failed follow-up thread ingestion. That gap allowed the reconciliation call to disappear while the regression test still passed.\n\nExercise a transient failure and require the hidden review comment to return through the pull-detail API with matching thread and timeline metadata, while confirming the provider review is published only once.\n\nValidation: focused reconciliation test; full internal/server package; golangci-lint; context-sync structural check.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
GitHub can return conversation-comment IDs that do not safely match through the legacy databaseId field. When moderation and REST comments use different identifiers, a minimized comment can appear visible.

Prefer fullDatabaseId for both comment records and visibility maps. The gated live check now executes the production initial-thread and paginated-comment queries against a real GitHub review thread, so schema support is verified by GitHub rather than inferred from stubs.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
Bulk sync could publish a complete conversation-comment snapshot without current inline review threads, then mark the pull request detail as fresh. That left hidden review replies stale until another detail refresh.

Load the initial conversation-comment and review-thread connections in one GitHub GraphQL snapshot. Keep later conversation pages on GraphQL so comment content and moderation share one authority, and leave detail stale whenever either discussion family is incomplete.

GitHub's live API accepted the production page-size query and returned both connections for PR #830. This verifies the schema and query-complexity limit beyond local stubs.

Triage finding fixed: #818 (comment)

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
Inline review replies were synchronized without contributing their provider timestamps to last_activity_at. A newer reply could therefore leave dashboard ordering and warm refresh scheduling anchored to older activity.

Use GitHub's GraphQL creation and update times in both bulk and detail-thread snapshots, without inventing local timestamps. The combined GraphQL fixture now crosses synchronization, SQLite, and the pull-detail API, including incomplete-thread preservation.

Live GitHub validation confirmed that both review-thread query forms return the timestamps. This fixes local roborev job 10346 and #818 (comment).

Validation: full internal/github and internal/server packages; focused combined GraphQL/API regressions; gated live GitHub GraphQL test; golangci-lint.

Generated with Codex

Co-authored-by: Codex <noreply@openai.com>
The frontend refactor moved runtime ownership and generated API contracts into the new application package. Direct component rendering no longer supplies the required Effect runtime, and regenerating the old UI schema leaves the compiled frontend unaware of review-thread moderation metadata.

Keep the privacy regressions on the shared application-runtime harness and move the generated contract change to the artifact the frontend now compiles.

Validation: focused frontend tests; frontend formatting, lint, Svelte checks, and Effect diagnostics; full internal/github and internal/server tests; live GitHub GraphQL validation; golangci-lint; context-sync structural check.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk
mariusvniekerk force-pushed the t3code/show-hidden-comments-collapsed branch from 42c3e86 to 58c5b9f Compare August 9, 2026 16:44
@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (58c5b9f)

Review found two medium-severity activity timestamp issues that can leave pull request ordering and refresh scheduling stale.

Medium

  • Locations: internal/server/pullapi/diff_review_handlers.go:869, internal/server/pullapi/routes.go:1161
    Successful review publication and direct thread replies persist inline events without updating last_activity_at. Since successful ingestion schedules no follow-up sync, dashboard ordering and warm-refresh scheduling can remain stale.
    Fix: Atomically update the merge request’s derived activity using provider timestamps when persisting inline comments, and add API/SQLite end-to-end assertions for publication and reply paths.

  • Location: internal/github/sync.go:10072
    Review-thread reconciliation seeds activity calculation with current.LastActivityAt, making the timestamp monotonic. When the newest inline comment is deleted, its event and thread are removed, but its timestamp remains indefinitely as the merge request’s latest activity.
    Fix: Recompute activity from the parent and retained non-inline datasets before applying timestamps from the newly fetched authoritative thread set. Add an end-to-end regression test covering deletion of the latest inline comment.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 11m49s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant