Rework commit deploy indicators on service charts#138
Conversation
There was a problem hiding this comment.
ℹ️ No critical issues — two minor cleanup observations below.
Reviewed changes — the rework of the service-detail charts' commit/deploy indicators from a per-reference-line marker into a dedicated, unit-tested overlay module.
- New
commit-markers/module —marker-layout.tsholds pure geometry (buildCommitMarkersfirst-seen-per-commit + bucket snapping,layoutMarkerLabelsgreedy non-overlapping sweep,estimateLabelWidth,shortLabel) kept React-free and covered bymarker-layout.test.ts;commit-markers-layer.tsxis the recharts overlay (hoverable chip row that overflows above the plot, per-group hover timers, anchoredMarkerCard);use-commit-markers.tsxwires releases→markers and resolves the representative SHA to a message subject in the background. - Richer hover card —
commit-sha-hover-card.tsxexportsCommitDetailBody(with acompactvariant) and addsCommitListBody/CommitListRow*for clustered deploys plusCommitPlainfor non-resolvable refs;deriveAvatarUrl,formatExact,formatRelativeShortextracted. - Tooltip suppression —
packages/uichart.tsxgainsChartTooltipSuppressionProvider/useSuppressChartTooltip(id-keyed) so a marker card quiets the data tooltip across a synced grid, keeping the tooltip mounted-transparent so its follow transition resumes rather than snapping from the origin. - Chart surface change —
chart-types.tsdropsChartReferenceLine+referenceLines/renderReferenceMarkerand addsoverlay,yAxisWidth, andMARKER_OVERLAY_CLASS; the four time-series charts render{overlay}and honoryAxisWidth;metrics-grid.tsxthreads both through and$serviceName.tsxpinsyAxisWidth={72}. - API + removals —
ServiceDetailOverviewResult.releaseswidened tostringand passed raw (snapping handles format); removedcommit-marker.tsx,release-markers.ts(+test),_shared/reference-markers.tsx, and the web wrappergetServiceReleasesTimeline(all references grep-clean).
ℹ️ Orphaned serviceReleases endpoint after the web wrapper removal
This PR removes getServiceReleasesTimeline, the only web-side consumer of the standalone serviceReleases HTTP endpoint. The releases data the charts need now rides along on serviceDetailOverview, so the dedicated endpoint, its request schema, and the atom-client method it backs are dead public surface on the web side.
The underlying ClickHouse query is still shared, so nothing breaks — but the standalone HTTP route is now unreachable from the app.
Technical details
# Orphaned `serviceReleases` endpoint
## Affected sites
- `apps/api/src/routes/query-engine.http.ts:445` — standalone `serviceReleases` handler; no web caller remains (grep for `.serviceReleases(` under `apps/web` returns nothing).
- `packages/domain/src/http/query-engine.ts:242` / `:1297` — `ServiceReleasesRequest` class + `POST /service-releases` endpoint definition, now only reachable by the dead handler.
- KEEP: `packages/query-engine/src/ch/queries/services.ts:130` `serviceReleasesTimelineQuery` — still used by the `serviceDetailOverview` handler (`query-engine.http.ts:561`). Do NOT remove this.
## Required outcome
- Either drop the now-unreachable standalone `serviceReleases` endpoint + `ServiceReleasesRequest` + atom-client method, or leave a note documenting it as an intentionally-kept public API.
## Open questions for the human
- Is the standalone `serviceReleases` endpoint a supported public API surface (e.g. consumed outside this repo), or purely an internal helper that can be deleted now that the overview query subsumes it?ℹ️ Nitpicks
firstLineis defined identically in bothapps/web/src/components/vcs/commit-markers/use-commit-markers.tsx:11andapps/web/src/components/vcs/commit-sha-hover-card.tsx:289. The latter already exportscommitQueryAtom/isResolvableSha, so exportingfirstLinefrom it and importing would drop the duplicate.
@v0 or keep the SHA fresh with Dependabot | Fix it ➔ | View workflow run | Using Claude Opus | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the aa3868e "some cleanup" commit on top of the previously-reviewed overlay rework. These are all polish/consolidation, including a fix for the prior review's firstLine duplication nit.
- Deduped
firstLine—commit-sha-hover-card.tsxnow exportsfirstLineanduse-commit-markers.tsximports it, dropping the duplicate helper flagged in the prior review. - Consolidated relative-time formatting —
formatRelativeShortandformatRelativemerged into a singleformatRelative(epochMs, { short }); default keeps the"3h ago"form with a seconds tier and"just now"floor,shortkeeps the"3h"/"now"form. Both call sites updated. - Capped the cluster badge —
commit-markers-layer.tsxrenders+{Math.min(badge, 99)}so a 100+ commit bucket can't exceed the fixedBADGE_PXreserved width (estimateLabelWidthuses the same constant, so layout and render stay in agreement). - Popover theme tokens — hover-card / chip surfaces switched from
bg-background/foregroundtobg-popover/popover-foreground(plus abackdrop-blur-smand a movedcursor-pointer). - Extracted
overlayChartClassName—chart-types.tsgains a shared helper replacing the inlineoverlay ? ... : classNameternary across the apdex / error-rate / throughput / latency charts; behavior is identical.
The consolidated formatRelative and the overlayChartClassName extraction are behavior-preserving, and the badge cap is self-consistent with the layout width estimate. The orphaned serviceReleases endpoint noted in the prior review is unchanged by this commit and remains an open question for the author.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Claude Opus | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the dc959ac..061c3f3 "more fixes" commit on top of the previously-reviewed overlay rework. Two substantive moves and one cleanup, all sound.
- Avatar derivation moved frontend → backend —
GithubProvider.tsgainsgithubAvatarUrl(htmlUrl, login), which resolves<host>/<login>.png?size=64against the commit's own html URL (so github.com and GitHub Enterprise both stay correct) and returns null when there's no login or the URL can't be parsed. It's wired into bothnormalizeFetchedCommit(RESTavatar_urlfallback) and the push-webhook path (previously hardcodednull), and the web-sidederiveAvatarUrlfallback is dropped so both hover-card call sites readcommit.authorAvatarUrldirectly. - Avatar test coverage — three new
vcs.test.tscases assert the github.com host, the GHE host, and the null-login fallback for push commits. - Tooltip suppression timing — in
commit-markers-layer.tsx, suppression andblockChartnow key offopenKeyrather thanhoverKey, so merely passing over a dash no longer blinks the chart's data tooltip away; only an opened card quiets it.hoverKeystill drives the card'sactivehighlight, so the hover affordance is intact. - Removed orphaned web wrapper —
getServiceReleasesTimelineand its now-unusedServiceReleasesRequestimport are gone fromservices.ts, partially addressing the prior review's orphaned-endpoint observation on the web side.
The avatar host derivation is correct (origin taken from the commit URL, not hardcoded), the suppression change is internally consistent, and the removals are grep-clean. The standalone serviceReleases HTTP endpoint + ServiceReleasesRequest domain class remain (this commit removed only the last web wrapper), but that's the unchanged carry-forward from the prior review and stays an open question for the author — not a new issue.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Claude Opus | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the 061c3f3..f1f2030 "add commit sha cache" commit on top of the previously-reviewed overlay rework. A single, well-scoped caching fix to the per-SHA commit-detail query atom, plus a guard test.
- Per-SHA atom now memoized + TTL'd —
commitQueryAtomincommit-sha-hover-card.tsxchanges from a bare(sha) => MapleApiAtomClient.query(...)intoAtom.family((sha) => ...query(..., { timeToLive: COMMIT_DETAIL_TTL_MS })). TheAtom.familystring-key memoization is what actually lets the prefetch subscriber, the popup body, the deploy markers, and the commit-list rows resolve to one shared atom — one in-flight fetch, one cached result — instead of each minting a fresh request object and refetching. - Idle TTL —
COMMIT_DETAIL_TTL_MS(5 min) is threaded astimeToLive, whichAtomHttpApimaps toAtom.setIdleTTLfor finite durations. Re-subscribing resets the timer, so a commit in active view never expires while truly-idle atoms are swept — bounding family growth. - Memoization contract test — new
commit-sha-hover-card.test.tsx(jsdom) asserts same SHA → identical atom instance and distinct SHA → distinct instance, the exact invariant the bare-function implementation violated.
The timeToLive → setIdleTTL mapping checks out (AtomHttpApi.ts: finite → idle TTL, infinite → keep-alive), so the documented "resets on re-subscribe, swept after last reader unmounts" behavior is accurate and the family won't leak. The standalone serviceReleases HTTP endpoint flagged in the initial review is untouched by this commit and remains the author's open call — not a new issue.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Claude Opus | 𝕏
| useEffect(() => { | ||
| const text = Result.builder(result) | ||
| .onSuccess((commit) => firstLine(commit.message)) | ||
| .orElse(() => null) | ||
| if (text) onResolved(sha, text) | ||
| }, [result, sha, onResolved]) |
There was a problem hiding this comment.
this should be a useCallback instead of this and build diff this seems hacky i think

Summary
Reworks the commit/deploy indicators on the service dashboard charts into a dedicated, unit-tested overlay layer. The old single-marker approach (
commit-marker.tsx,release-markers.ts,reference-markers.tsx) is replaced by acommit-markers/module that separates pure layout geometry from the React/recharts overlay.What changed
commit-markers/module (apps/web/src/components/vcs/commit-markers/):marker-layout.ts— pure geometry/derivation (bucket → marker, label layout, sha shortening), kept free of React so it unit-tests cleanly. Covered bymarker-layout.test.ts.commit-markers-layer.tsx— thin recharts overlay rendering dashes + labels, with hover-card coordination and zIndex layering.use-commit-markers.tsx— hook wiring commit data into the layer.commit-sha-hover-card.tsx) —CommitDetailBody/CommitListBodyfor single vs. clustered commits.packages/uichart.tsxgains tooltip-suppression coordination so the commit card and the data tooltip don't fight.commit-marker.tsx,release-markers.ts(+test), and_shared/reference-markers.tsx.Notes
scripts/ingest-dummy.ts,ingest:dummyscript) used to test this was intentionally left out of the PR.🤖 Generated with Claude Code