LearnNewsExamplesServices
Frontmatter
id10110
titleRemove commentsTotal sentinel sweep — derive count from timelineItems
stateClosed
labels
bugairefactoringregressionarchitectureperformance
assigneestobiu
createdAtApr 19, 2026, 11:40 PM
updatedAtApr 20, 2026, 12:15 AM
githubUrlhttps://github.com/neomjs/neo/issues/10110
authortobiu
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
blockedBy[]
blocking[]
closedAtApr 20, 2026, 12:15 AM

Remove commentsTotal sentinel sweep — derive count from timelineItems

Closed v13.0.0/archive-v13-0-0-chunk-5 bugairefactoringregressionarchitectureperformance
tobiu
tobiu commented on Apr 19, 2026, 11:40 PM

The Problem

Startup-sync latency for the gh-workflow MCP server regressed severely in PR #10093 (ticket #10092). The detectStaleCommentsCounts sentinel sweep — added to catch comment-deletion drift that slips past the updatedAt gate — runs 37 serial GraphQL round trips per runFullSync (3,669 tracked issues / 100 batch size), adding ~11–18 seconds of wall-clock per sync on every invocation. This pushes the MCP stdio handshake over Claude Code's registration budget, causing intermittent UI invisibility of the server and, when the client kills the process mid-sync, truncating the subsequent git pull --rebase --autostash and wedging the main checkout (observed 2026-04-19 at 22:42PM — separate recovery required).

The sweep's raison d'être was narrow to begin with: GitHub does not bump issue.updatedAt when a comment is deleted, so comment deletion on an issue with no subsequent activity stays locally rendered indefinitely. In practice this matters only for closed/quiet issues — an archival-accuracy concern, not an operational one.

The Architectural Reality (the actual oversight)

The gh-workflow sync currently stores commentsTotal in metadata via issue.comments?.totalCount ?? 0 at ai/mcp/server/github-workflow/services/sync/IssueSyncer.mjs:450 and :593 — a scalar sourced from GitHub's dedicated comments { totalCount } GraphQL field.

Historical context: the first versions of this MCP server didn't fetch timelines — comments came from a dedicated endpoint. When unified timelines were added later (primarily for the portal app's ticket rendering) the sync converged on timelineItems as the authoritative per-issue event source, with #10090 adding pagination-exhaust so timelineItems.nodes is the complete set. Sourcing commentsTotal from a separate scalar is a pre-timeline-era leftover — every comment already lives in timelineItems.nodes as an ISSUE_COMMENT typed node. Counting them is a JS loop over data already in memory. The #10092 sentinel is secondary infrastructure built on top of that oversight.

Fix

  1. Remove the sweep and its supporting infrastructure:
    • SyncService.runFullSync — delete lines 107-115 (detectStaleCommentsCounts call + stale-count refetch wiring)
    • IssueSyncer.detectStaleCommentsCounts — delete method
    • issueQueries.buildIssueTotalsBatchQuery — delete function
    • config.template.staleCommentsBatchSize — delete knob
  2. Derive commentsTotal from timelineItems at IssueSyncer.mjs:450 and :593 — post-#exhaustTimelineItems so authoritative. One source of truth: the metadata field cannot disagree with what's rendered in the markdown.
  3. Drop comments { totalCount } sub-selection from FETCH_ISSUES_FOR_SYNC and FETCH_SINGLE_ISSUE — no longer consumed, minor GraphQL complexity saving.
  4. Keep refetchIssuesByNumber — still the healing primitive for #10090 pagination recovery and on-demand fixup.
  5. Update IssueSyncer.spec.mjs — remove the three stale-count cases (lazy seed / no-drift / drift detection); add assertion that commentsTotal derived at write time matches timeline's ISSUE_COMMENT node count.

Accepted Blind Spot

After this change, a deleted comment on an issue that subsequently receives zero activity (no new comments, no label/milestone changes, no reactions) will remain locally rendered until something eventually bumps the issue's updatedAt. This is explicitly accepted: the cost of the sentinel sweep (~15s/sync, every sync) is not justified by the narrow case it catches. Agents needing audit-grade accuracy can invoke the existing refetchIssuesByNumber SDK endpoint on demand.

Avoided Traps

  • Extending the detector pattern (#10094). With CommentTotalsDetector removed, only #7948's relationship-event scan remains, and that's intrinsic to the fetch path — not a sweep. The detector abstraction has zero sweep instances left to unify. #10094 should be closed as superseded when this PR lands.
  • Scheduled / periodic sweep. Still pays the cost, just on a timer. Duct tape on a marginal design choice.
  • Transport-decoupling of startup sync. Previously discussed and rejected — consistency with memory-core and knowledge-base (both block transport on sync completion) is a deliberate ecosystem contract.

Acceptance Criteria

  • runFullSync no longer invokes detectStaleCommentsCounts; method + buildIssueTotalsBatchQuery + staleCommentsBatchSize config removed
  • commentsTotal derived from timelineItems.nodes.filter(n => n.__typename === 'IssueComment').length at both write sites (IssueSyncer.mjs:450 and :593)
  • comments { totalCount } sub-selection removed from FETCH_ISSUES_FOR_SYNC and FETCH_SINGLE_ISSUE
  • IssueSyncer.spec.mjs stale-count cases removed; new assertion covers timeline-derived count correctness
  • runFullSync logs no longer reference staleCounts
  • Measured boot-sync wall-clock restored to pre-#10093 baseline

Related

  • Reverses approach of: PR #10093 (ticket #10092)
  • Supersedes: #10094 (detector interface — close on merge)
  • Leaves intact: #10090 (pagination stays; intrinsic to fetch path), #7948 (relationship-event scan stays; intrinsic to pullFromGitHub)

Origin Session ID

07f601dc-353a-44d2-a373-18da2a0d305a

tobiu added the bug label on Apr 19, 2026, 11:40 PM
tobiu added the ai label on Apr 19, 2026, 11:40 PM
tobiu added the refactoring label on Apr 19, 2026, 11:40 PM
tobiu added the regression label on Apr 19, 2026, 11:40 PM
tobiu added the architecture label on Apr 19, 2026, 11:40 PM
tobiu added the performance label on Apr 19, 2026, 11:40 PM
tobiu assigned to @tobiu on Apr 19, 2026, 11:41 PM
tobiu referenced in commit 73d9e01 - "fix(github-workflow): remove commentsTotal sentinel sweep — derive from timelineItems (#10110)" on Apr 19, 2026, 11:52 PM
tobiu cross-referenced by PR #10111 on Apr 19, 2026, 11:53 PM
tobiu referenced in commit 55b4a44 - "style(github-workflow): drop stray blank line in frontmatter object (#10110)" on Apr 20, 2026, 12:00 AM
tobiu referenced in commit 087b2f8 - "fix(github-workflow): remove commentsTotal sentinel sweep (#10110) (#10111) on Apr 20, 2026, 12:15 AM
tobiu closed this issue on Apr 20, 2026, 12:15 AM
tobiu cross-referenced by PR #10114 on Apr 20, 2026, 1:02 AM