Skip to content

fix(validation): reject unsupported counter windows - #36

Merged
alice-viola merged 1 commit into
masterfrom
fix/validate-counters-window
Sep 7, 2026
Merged

fix(validation): reject unsupported counter windows#36
alice-viola merged 1 commit into
masterfrom
fix/validate-counters-window

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Problem

counters.windowSeconds was exposed as configurable, but the runtime always slept for 60 seconds, keyed rows to minute boundaries, and served them through minute-based history APIs. A declaration such as windowSeconds: 30 was accepted while silently producing 60-second data.

Fix

  • reject every counter window other than the currently supported 60 seconds with a new counters-window validation problem
  • define the supported duration once and use it in both validation and the flush loop
  • correct the design text: roll-ups come from per-replica stage-counter deltas, not a per-graph streams job
  • document the fixed one-minute contract in the README

Review notes for Alice

This deliberately validates the implementation Gate has today instead of pretending arbitrary windows work. Supporting another duration is a storage/API migration: roll-up keys, history query semantics and UI minute ranges are all minute-based. Existing valid declarations (60 or omitted counters) are unchanged; an existing non-60 declaration will be refused on its next redeclare rather than continue lying about its resolution.

Verification

  • cargo test --workspace
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo fmt --check

@albertovincenzi

Copy link
Copy Markdown
Collaborator Author

This rule is retroactive on documents that are already in the store.

restore and reconcile both go through declare_from_storedeclare_locked(.., from_caller = false)validate_with, with no exception. A single Problem therefore means the graph is not registered on the next restart: its pushes answer 404 and its ingress queue fills behind one WARN line.

#66 draws the line — a stored document is refused only for nodes, paths or a name that cannot become a queue name and a kv key, and everything else is logged and kept running. Suggest basing this PR on it, or merging it first.

For #34 specifically this is not hypothetical: airbnb/content and airbnb/messages in crates/core/tests/testdata/v1_channel_go_graphs.json carry only conditional budgets, and the passthrough remedy in migrate.rs never runs again once a document has been re-saved in v2 shape.

https://claude.ai/code/session_012K8u7BEJyd6nDNMCQAgH3z

@alice-viola
alice-viola merged commit bbb71ef into master Sep 7, 2026
1 check passed
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