Skip to content

chore(alerts): drop the alert chart's transitional flat points - #1005

Merged
JeremyFunk merged 3 commits into
mainfrom
chore/drop-alert-chart-points-fallback
Sep 23, 2026
Merged

JeremyFunk merged 3 commits into
mainfrom
chore/drop-alert-chart-points-fallback

Conversation

@JeremyFunk

@JeremyFunk JeremyFunk commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #953, which kept a skew bridge on /v2/share/alert-chart: the api emitted the flat points alongside series, and web fell back to points when series was missing.

The bridge isn't needed any more. Both Workers have deployed from 351a71321, and that commit contains #953 (373dc96c1):

  • Deploy PRD run 35910457310 (351a71321) succeeded, and every job in it succeeded, including deployment-gate, deploy-prd and deploy-prd-eu. Web's Worker reconciled in that run.
  • curl https://api.maple.dev/health returns x-maple-revision: 351a71321f9aeec5d92f6959e9c4bb739f4967a7.

Changes:

  • api: stops emitting points on the alert chart response.
  • domain: removes the optional ChartTimeseries.points and its skew comment.
  • web: cardFor reads series directly and is typed as ShareChartResponse. It is no longer exported, since the skew test was the only thing importing it, and that test pair is gone.

Nothing else reads points on this response. The iOS OpenAPI spec doesn't include ChartTimeseries, so it doesn't need regenerating.

Tests: apps/web src/og (44), packages/domain src/http (263), and the packages/widgets static-chart golden tests (6) all pass.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Changes
    • Shared alert-chart responses now provide time-series data through the series field and no longer include the legacy flat points field.
    • Chart image rendering now relies on the current series format; older chart responses that only contain flat points data are no longer supported.

#953 emitted the flat `points` alongside `series` on /v2/share/alert-chart,
and web fell back to it, to survive an api/web deploy skew. Both Workers have
since deployed from 351a713, which contains #953, so the fallback is dead.
@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 15 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 81d8889d-22d5-41ce-942c-f1107130fb42

📥 Commits

Reviewing files that changed from the base of the PR and between cca8b0b and 139612c.

📒 Files selected for processing (4)
  • apps/api/src/routes/v2/share.http.ts
  • apps/web/src/og/chart-image.test.ts
  • apps/web/src/og/chart-image.ts
  • packages/domain/src/http/share-chart.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: b6502155-b587-4dde-b1e9-924594f69fbb

📥 Commits

Reviewing files that changed from the base of the PR and between 0a8416f and cca8b0b.

📒 Files selected for processing (4)
  • apps/api/src/routes/v2/share.http.ts
  • apps/web/src/og/chart-image.test.ts
  • apps/web/src/og/chart-image.ts
  • packages/domain/src/http/share-chart.ts
💤 Files with no reviewable changes (2)
  • apps/api/src/routes/v2/share.http.ts
  • packages/domain/src/http/share-chart.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The chart timeseries contract and alert-chart response no longer include the flat points field. Chart image code uses ShareChartResponse and reads timeseries data from series; tests for the legacy response shape are removed.

Changes

Share chart payload cleanup

Layer / File(s) Summary
Remove flat points from chart responses
packages/domain/src/http/share-chart.ts, apps/api/src/routes/v2/share.http.ts
ChartTimeseries and the alertChart response no longer include the optional flat points field.
Use the shared chart response type
apps/web/src/og/chart-image.ts, apps/web/src/og/chart-image.test.ts
Chart image code uses ShareChartResponse and reads timeseries data from series. Tests for the older flat points response shape are removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to cca8b

The alert-chart response and chart-image renderer now use the same series format. No actionable merge-blocking risk is established beyond normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing the transitional flat points from alert charts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@maple-review-bot

maple-review-bot Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Maple observability review: 100/100

Excellent · Observability looks complete · reviewed b4ecadf

Score Critical Warnings Notes Changes observable
100/100 0 0 0 3 of 3

This PR is purely subtractive: it removes the transitional flat points field from the alert-chart response, the matching optional field on ChartTimeseries, and the web-side ?? points fallback in cardFor. It adds no new entrypoint, outbound call, background work, error path, log statement, metric or attribute key, so there is nothing new that needs instrumentation; the touched paths keep their existing spans and the pre-existing 404-on-failure handling is unchanged. One small side benefit for observability: dropping the silent fallback means a response without series can no longer be quietly mis-drawn, and the author's deploy evidence (both Workers past 351a713, which contains #953) means the removal is safe in both deploy orders.

What was reviewed
Change Kind Observable Evidence
GET /v2/share/alert-chart response body (apps/api/src/routes/v2/share.http.ts) http route response (api) yes The diff only deletes the transitional points field from the response body (and its skew comment). The route itself, its span and any existing instrumentation are untouched; no new unit is added.
OG alert-chart render path, renderChartImage/cardFor (apps/web/src/og/chart-image.ts) outbound call + rendering (web) yes Changes are type tightening plus removing the ?? points fallback; the existing api.fetch call, the try/catch that maps failures to 404, and the rest of the render flow are pre-existing context, not added by this diff. No new outbound call, error path, log, metric or attribute key is introduced.
ChartTimeseries (packages/domain/src/http/share-chart.ts) domain schema/contract yes Pure contract change: the optional flat points field is removed from the Schema.Class. There is no runtime work here to instrument, and no attribute keys are added or renamed.

Score: 100, minus 25 per critical finding, 10 per warning and 2 per note. Check ids refer to Maple's instrumentation audit. Updated on every push.

@JeremyFunk
JeremyFunk merged commit 346c3a5 into main Sep 23, 2026
40 of 41 checks passed
@JeremyFunk
JeremyFunk deleted the chore/drop-alert-chart-points-fallback branch September 23, 2026 22:51
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