Skip to content

fix(validator): bound the data_visualization consistency checks - #86

Merged
StephenTangCook merged 1 commit into
mainfrom
claude/quadratic-categories-labels-loop-9homkr
Sep 1, 2026
Merged

fix(validator): bound the data_visualization consistency checks#86
StephenTangCook merged 1 commit into
mainfrom
claude/quadratic-categories-labels-loop-9homkr

Conversation

@StephenTangCook

@StephenTangCook StephenTangCook commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

checkDataVisualizationConsistency counted each category with labels.filter(...), rescanning the whole label array once per category — O(categories × labels) per series. A 217 KB payload (5,000 categories, 5,000 data points) cost ~270 ms of CPU where a normal payload of the same size costs under a millisecond, on a public unauthenticated endpoint.

Two things compounded it:

  • The helpers ran whether or not the schema passed. In validateBlockKit the Ajv result only gated an errors.push(), so the schema's own maxItems caps (series: 6, data: 20) never constrained the path — a payload reached the quadratic loop precisely by being schema-invalid.
  • axis_config.categories had no maxItems while both its siblings did, and helper results were aggregated with errors.push(...helper()). 25,000 categories (212 KB, under the API's 256 KB cap) produced ~150,000 error strings and the spread exceeded V8's argument limit — an uncaught RangeError: Maximum call stack size exceeded, which the worker turned into an unhandled error rather than a response.

Reproduced on main before the change and re-measured after:

payload before after
5,000 categories × 5,000 labels (216.6 KB) 274 ms 8 ms
25,000 categories × 6 series (212.1 KB) RangeError verdict, ~30 ms

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing behavior to change)
  • Schema update (changes to src/slack-block-kit.schema.json)
  • Docs / chore (no runtime change)

Changes

  • check-data-visualization-consistency.ts — count the labels once into a Map, then look each category up in O(1): O(categories + labels) instead of O(categories × labels).
  • check-data-visualization-consistency.ts — stop after 100 mismatch messages per series and summarize the rest. The helper is exported on its own via /helpers, where no schema has run, so nothing upstream bounds its input; this keeps an unbounded payload from becoming an unbounded array (150,000 strings → 606 in the reported case).
  • slack-block-kit.schema.json — cap axis_config.categories at 20 items, alongside the existing caps on series (6) and data (20).
  • validate-block-kit.ts — skip the cross-payload helpers once the schema has rejected the payload. This is the part worth the most attention: it closes the whole "bypass the schema's bounds by being schema-invalid" path rather than this one instance of it.
  • validate-block-kit.ts — append helper results with a loop (pushAll) instead of a spread, for all eleven helpers.
  • worker/src/validate.ts — catch anything thrown out of validateBlockKit and return a structured 500, so an unanticipated throw is JSON with CORS and rate-limit headers rather than a bare runtime error. Documented as a 500 response in the OpenAPI spec.

Three behavior changes reviewers should weigh:

  1. An invalid payload's errors[] no longer mixes schema and caveat-helper messages — schema errors come back on their own, and you re-run to see the cross-payload ones. The valid verdict is unchanged either way, since schema errors alone already made it false. Documented in the README's API reference.
  2. categories now generates a tuple union in types.generated.ts, as every other capped array in this schema already does, so a TypeScript consumer assigning a plain string[] will need a typed literal. Kept as a fix: (patch) deliberately: consumers on ^0.1.x would not pick up a minor bump, and nothing about the runtime verdict changes. Happy to re-tag it as breaking if you would rather have the signal than the automatic upgrade.
  3. Payloads with more than 20 categories now fail the schema. They were already invalid — a series' data array is capped at 20 and must carry exactly one point per category, so more than 20 categories was never satisfiable — so no previously-valid payload changes verdict.

One test payload was corrected rather than adapted: worker/test/index.test.ts asserted the surface-compat rule using an alert block with a title field, which the schema rejects (alert requires text). It only passed because helpers ran on schema-invalid input; it now uses a valid alert block and exercises the rule it names.

Testing

  • pnpm test passes — 521 tests (6 new)
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm validate-schema passes (if the schema changed)
  • Added or updated tests under test/ covering the change

Each new test was run against the pre-fix source to confirm it fails there: the linearity guard (4,017 ms vs. a 1,000 ms budget), the per-series cap (24,999 errors vs. 101), the schema cap, the helper short-circuit, and the oversized-chart case (RangeError). worker/ tests pass too (34), including a new end-to-end check that a 25,000-category body returns a verdict instead of a 500.

Schema changes

  • Verified against the Slack Block Kit reference — the cap follows from the documented data limit of 20 and the one-point-per-category rule
  • Backwards compatible (existing valid payloads still validate)
  • If breaking, called out in the summary and a release note is planned

Checklist

  • Commit messages follow Conventional Commits (required for release-please)
  • Public API changes are reflected in README.md
  • No secrets, tokens, or sample tenant data committed

Generated by Claude Code


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

`checkDataVisualizationConsistency` counted each category with
`labels.filter(...)`, rescanning every label once per category — O(categories
× labels) per series. A 217 KB payload (5,000 categories, 5,000 data points)
cost ~270 ms of CPU where a normal payload of the same size costs under a
millisecond. `validateBlockKit` ran the cross-payload helpers whether or not
the schema had passed, so the schema's own `maxItems` caps never constrained
that path: a payload reached the quadratic loop precisely by being
schema-invalid.

The same helper emitted one error per series × uncovered category, and
`validateBlockKit` aggregated helper results with `errors.push(...helper())`.
25,000 categories (212 KB, under the API's 256 KB cap) produced ~150,000
strings, and the spread exceeded V8's argument limit — an uncaught
`RangeError: Maximum call stack size exceeded`.

- count the labels once into a `Map`, then look each category up in O(1):
  O(categories + labels) instead of O(categories × labels)
- stop after 100 mismatch messages per series and summarize the rest, so
  unbounded input reaching the helper directly through `/helpers` can't turn
  into an unbounded array
- cap `axis_config.categories` at 20 items, alongside the existing caps on
  `series` (6) and `data` (20) — a series must carry exactly one data point
  per category, so more than 20 was never satisfiable
- skip the cross-payload helpers once the schema has rejected the payload,
  closing the whole "bypass the schema's bounds by being schema-invalid"
  path rather than this one instance of it
- append helper results with a loop rather than a spread
- catch anything thrown out of `validateBlockKit` in the worker's
  `/v1/validate` handler and return a structured 500

Measured on the reported payloads: 5,000 × 5,000 goes from ~270 ms to
~8 ms, and 25,000 categories returns a verdict instead of throwing.

Behavior changes worth knowing about:

- an invalid payload's `errors[]` no longer mixes schema and caveat-helper
  messages; schema errors come back on their own. The `valid` verdict is
  unchanged — schema errors alone already made it `false`.
- `categories` now generates a tuple union in `types.generated.ts`, as every
  other capped array in the schema already does, so TypeScript consumers
  assigning a plain `string[]` will need a typed literal. Released as a
  patch deliberately: consumers on `^0.1.x` would not pick up a minor bump,
  and no runtime verdict changes.
- payloads with more than 20 categories now fail the schema. They were
  already invalid — with 20-item `data` arrays they could never cover more
  than 20 categories — so no previously-valid payload changes verdict.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MAfoobRJmr4GAMETtDaveE
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
slack-block-kit-validator e87190f Commit Preview URL

Branch Preview URL
Sep 01 2026, 12:30 AM

@StephenTangCook
StephenTangCook merged commit 837a96c into main Sep 1, 2026
14 checks passed
@StephenTangCook
StephenTangCook deleted the claude/quadratic-categories-labels-loop-9homkr branch September 1, 2026 00:37
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.

2 participants