Tune CodeRabbit reviews from a 90-day audit

.claude/skills/build-plugin/references/data-streams.md

@@ -58,6 +58,11 @@ So the rule is **push it down**: if the API can group/filter/sort, let it; other
 
 The `cost` example below does exactly this: its optional `groupBy` `choiceChips` param **is** layer-1 push-down.
 
+**Filter and count in the API when the API caps its results.** Check which kind of cap an endpoint has:
+
+- **A cap on the total result** (e.g. a search that never returns more than 2,000 rows) can't be paged past. A count of the rows that came back, or a filter applied in the tile or a script after the request, silently undercounts: one shipped plugin showed 303 of 601 SLA breaches this way. The filter or count has to be an API parameter.
+- **A cap per page** is recovered by [paging](#pagination), provided the endpoint honours its paging parameters.
+
 ### Anti-pattern: five "Cost by X" streams
 
 A shipped plugin has five cost streams — `costByProject`, `costByService`, `costDailyTimeline`, `projectCosts`, `projectCostTimeline`. All five issue the **identical** request to `/v1/billing/charges` (same `getArgs`), and each `postRequestScript` does the same thing: parse the NDJSON, filter to `ChargeCategory === 'Usage'`, then group-and-sum by a different key (project / service / day) — two of them additionally filtering to a scoped project. They differ **only** by group key and scope, which is the "consolidate" column above.
@@ -254,7 +259,7 @@ result = scoped.map((r) => ({
 
     > ⚠️ Never mention dashboards, tiles, or import — every stream ends up in a dashboard somewhere, so saying so adds no information. This includes rephrasings like "used for import", "used by the X tile", "powers the X dashboard", "for monitoring X".
 
-- `tags` — required; title case (e.g. `"User"`, `"Security"`, `"Analytics"`). Keep to a small, meaningful set, reuse existing tags from other plugins/data streams. Do not use the plugin name as a tag.
+- `tags` — required on most streams; title case (e.g. `"User"`, `"Security"`, `"Analytics"`). Tags group related streams in the tile editor, so choose them across the whole plugin: not one tag shared by every stream, and not many tags that each cover a single stream. Reuse existing tags from other plugins/data streams where it makes sense. Do not use the plugin name as a tag.
 
 ---
 
@@ -388,6 +393,8 @@ Shows an **Apply** button instead of running on every config change. Use for exp
 
 The `paging` block in `config` controls how SquaredUp fetches multiple pages.
 
+Only declare paging an endpoint actually honours: check that its paging parameters change the response. If the endpoint ignores them, SquaredUp re-fetches the same page until the page cap and imports duplicates. `"mode": "none"` with the limit written in the README is the better choice there. Before adding paging to a stream with a post-request script, read the [per-page warning](#post-request-scripts).
+
 **No paging:**
 
 ```json
@@ -528,6 +535,8 @@ none
 
 `none` is only valid when `supportsNoneTimeframe` is set (see below).
 
+**Apply both ends of the range.** A stream that filters by timeframe must pass both the start and the end to the API (or clamp to both in its script). Filtering only on the start, or looping up to now instead of the end, looks right for "last 7 days" but over-returns for closed windows such as `lastMonth` or `lastQuarter`.
+
 JSON-only timeframe properties (not settable via the Save as data stream modal):
 
 ```json
@@ -788,7 +797,9 @@ Appending a related object's list property this way lets tiles filter and group
 
 ## Post-request scripts
 
-Scripts run after the HTTP response is received. Input is `data` (parsed JSON body). Set `result` to an array of row objects. (For a datasource-level script that runs _before_ every request — custom auth flows — see [Pre-request scripts](metadata.md#pre-request-scripts-custom-auth-flows) in metadata.md.)
+Scripts run after the HTTP response is received. Input is `data` (parsed JSON body). Set `result` to an array of row objects.
+
+> ⚠️ **On a paged stream the script runs once per page and sees only that page.** SquaredUp concatenates each page's `result`. Anything that needs the whole result set is wrong once there is a second page: de-duplicating, pairing related records (an alert and the event that clears it), totals, counts or unions. Turning paging on does not fix this, it causes it. Either derive the value from data within each record (one plugin used each alert's own link to its clearing event instead of searching the page for it), push the work to the API, or leave it to the tile's `group`/`aggregate`, which runs on the combined rows. (For a datasource-level script that runs _before_ every request — custom auth flows — see [Pre-request scripts](metadata.md#pre-request-scripts-custom-auth-flows) in metadata.md.)
 
 ### Wiring a script to a stream
 
@@ -923,6 +934,8 @@ Return actual JS number primitives for numeric columns — returning `"29.19"` (
 
 ### Deduplication pattern
 
+This de-duplicates within one response. On a paged stream it only removes duplicates within each page (see the warning under [Post-request scripts](#post-request-scripts)).
+
 ```javascript
 // dataStreams/scripts/myStream.js
 const seen = new Set();

.claude/skills/build-plugin/references/oob-content.md

@@ -126,7 +126,7 @@ Single `.dash.json` files reference as `"type": "dashboard"`. Folders map to sub
 
 The tile-level `"scope"` block above is for streams that are **natively scoped** through their `matches`. A stream that is unscoped at the definition level (`"matches": "none"`) but exposes an optional `objects` parameter — the consolidation pattern in [data-streams.md](data-streams.md#one-stream-per-shape) — auto-scopes on a perspective a different way: **bind the dashboard variable into the parameter** under `dataStream.dataSourceConfig`.
 
-The key under `dataSourceConfig` is the `name` of the stream's `objects` field. Given a stream with `"ui": [{ "type": "objects", "name": "project", ... }]`, the per-object perspective dashboard binds:
+The key under `dataSourceConfig` is the `name` of the stream's `objects` field. Stream parameters on a tile always go under `dataStream.dataSourceConfig`. The platform silently ignores anything placed under `dataStream.config`, so the tile runs with defaults and looks plausible but wrong. Given a stream with `"ui": [{ "type": "objects", "name": "project", ... }]`, the per-object perspective dashboard binds:
 
 ```json
 "dataStream": {
@@ -254,6 +254,8 @@ Common tiles, with the **exact** column names the chart's `field`s must use:
 - **Do not repeat the plugin name in dashboard names.** The name appears beneath the plugin name in the UI — "MyPlugin / Overview" is correct; "MyPlugin / MyPlugin Overview" is redundant.
 - **Give each dashboard a distinct name.** Perspective tabs sit next to each other — identical names are indistinguishable.
 - `"variables"` array supports **only one variable** per dashboard. Design each dashboard around a single object type.
+- **Every variable a tile references must be declared** in the dashboard's `"variables"` array, otherwise the binding resolves to nothing.
+- **Keep request counts low on overview tiles.** A tile over an `httpRequestScopedSingle` stream sends one request per object in scope, so a summary of 50 environments is 50 API calls every time the dashboard loads. (Other scoped streams can send all the objects in one request.) Prefer a single list call, or a property captured at import, for account-wide summaries; keep per-object streams for drilldown dashboards.
 - **A tile's `"timeframe"` is driven by its data stream, not by style.** If the tile's stream is **timeframe-independent** (`"timeframes": false` — a current-state / snapshot stream that ignores any range), set `"timeframe": "none"` on the tile. If the stream **supports timeframes**, **omit** `timeframe` so the tile tracks the dashboard's timeframe — setting `"none"` here pins the tile and stops it following the dashboard. (This is why most summary tiles, built on snapshot streams, carry `"none"`.)
 - All tile IDs (`"i"`) must be **genuinely random UUIDs** — generate with `node ".claude/skills/build-plugin/scripts/gen-uuids.js" [N]`. Never invent patterned UUIDs.
 
@@ -334,3 +336,11 @@ These are standalone tiles with no `dataStream`, `scope`, `activePluginConfigIds
 | `{{scopes.[ScopeName]}}`       | A scope by name from `scopes.json` |
 | `{{dataStreams.[streamName]}}` | The data stream's ID               |
 | `{{variables.[VariableName]}}` | The variable defined on a scope    |
+
+**Escape every other `{{…}}` with `{{{{raw}}}}`.** These tokens are substituted when the default content is installed, and any other `{{…}}` in a `.dash.json` is substituted too, normally to nothing. Text that SmartViz itself fills in at render time therefore has to be wrapped so it reaches the tile intact. Typical cases are `labelFormat`, `cartesianTooltipTemplate` and `statusGridSecondaryTemplate`:
+
+```json
+"labelFormat": "{{{{raw}}}}{{value}}{{{{/raw}}}}"
+```
+
+A tile whose label or tooltip comes out blank after install usually has an unwrapped template.

.claude/skills/deploy-plugin/SKILL.md

@@ -76,15 +76,15 @@ Downstream callers (e.g. Checkpoint A) should check the `valid` field to disting
 
 ## Versioning
 
-New plugins start at `1.0.0`. Use semver:
+New plugins start at `<N>.0.0`, where `N` matches the `v<N>/` folder (`1.0.0` in `v1/`, `2.0.0` in `v2/`). Use semver:
 
 | Change type | Bump |
 |---|---|
 | Bug fix, docs, icon, metadata tweak | PATCH (`1.0.x`) |
 | New stream, new optional config field, new default content | MINOR (`1.x.0`) |
 | Deleted/renamed stream, breaking config change | MAJOR (`x.0.0`) |
 
-Versions only matter at merge, so they are compared against `main`. A PR that modifies an existing plugin must end with a `version` higher than the one on `main` — that is **one bump for the whole PR, not one per commit**, so don't bump again on each review round. A brand-new plugin has nothing on `main` to compare against: leave it at `1.0.0` until it merges.
+Versions only matter at merge, so they are compared against `main`. A PR that modifies an existing plugin must end with a `version` higher than the one on `main` — that is **one bump for the whole PR, not one per commit**, so don't bump again on each review round. A brand-new plugin has nothing on `main` to compare against: leave it at `<N>.0.0`, matching its `v<N>/` folder, until it merges.
 
 **Breaking (MAJOR) changes — do not create a new major version without asking the user first.** It is often possible to avoid the break entirely. If a major version is genuinely needed:
 - Create a new versioned folder (e.g. `v2/`) rather than modifying `v1/`

.coderabbit.yaml

@@ -0,0 +1,147 @@
+# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
+#
+# CodeRabbit configuration. The review rules themselves live in REVIEW.md so
+# that humans, AI coding agents and CodeRabbit all work from one source; keep
+# this file to settings and to pointers into that document.
+language: en-GB
+tone_instructions: >-
+  Be concise and neutral. Lead with the impact on the plugin user. Prefer one
+  comment listing every affected file over one comment per file.
+
+reviews:
+  profile: chill
+  high_level_summary: true
+  collapse_walkthrough: true
+  changed_files_summary: true
+  sequence_diagrams: false
+  estimate_code_review_effort: false
+  related_prs: false
+  suggested_labels: false
+  suggested_reviewers: false # CODEOWNERS already routes reviews
+  poem: false
+  in_progress_fortune: false
+
+  # Approve once the latest commit is reviewed, its threads are resolved,
+  # pre-merge checks pass and merge risk is low. CodeRabbit also requests
+  # changes while blocking findings are open. It is not a code owner, so it
+  # does not stand in for a CODEOWNERS review.
+  auto_approvals:
+    enabled: true
+    max_merge_risk: low
+    # Authors can't use "@coderabbitai approve" on their own PRs.
+    allow_author_approval: false
+    overrides:
+      # Never approve changes to the reviewer's own rules, CI, or the agent
+      # skills that outside users install.
+      - paths:
+          - ".coderabbit.yaml"
+          - "REVIEW.md"
+          - "AGENTS.md"
+          - "CLAUDE.md"
+          - ".claude/**"
+          - ".github/**"
+        enabled: false
+  merge_risk_instructions: |
+    Higher risk:
+    - Changes to a plugin that already exists on main that remove or rename
+      data streams, source types, object IDs or UI fields, since they break
+      customers' dashboards or force a reimport.
+    - Changes to authentication: preRequest.js, OAuth or token handling,
+      base.config headers and URL patterns.
+    - Import changes (indexDefinitions, import streams, pageSize), which can
+      fail or overload a customer's import.
+    - Anything that sends credentials or tenant data somewhere new.
+    Lower risk:
+    - A new plugin, which affects no existing customer.
+    - Dashboard layout, wording, README and help text changes.
+    - A version bump with a small, contained fix.
+
+  auto_review:
+    enabled: true
+    drafts: true
+    # Plugins often go through many small review rounds; the default of 5
+    # left final commits unreviewed on several PRs.
+    auto_pause_after_reviewed_commits: 15
+
+  path_instructions:
+    - path: "plugins/**"
+      instructions: |
+        Apply REVIEW.md, including the "How to review" and "Platform facts"
+        sections. Before asking for a version bump, check whether
+        plugins/<Name>/v<N>/metadata.json is an added file in this PR. If it
+        is, the plugin is new: it must be <N>.0.0 and needs no bump.
+        .claude/skills/build-plugin/references/ is the authority on platform
+        behaviour (shapes, paging, timeframes, imports, dashboards). Check it
+        before claiming a field, shape or value is unsupported.
+        Do not report Biome's "Illegal return statement" on preRequest.js or
+        on scripts under dataStreams/scripts/: the platform runs each script
+        inside a function, so a top-level return is valid.
+    - path: "plugins/**/dataStreams/**"
+      instructions: |
+        Check scripts, paging, timeframes and capped results against
+        .claude/skills/build-plugin/references/data-streams.md, in particular
+        its warnings on per-page post-request scripts, timeframe bounds and
+        APIs that cap their results.
+    - path: "plugins/**/defaultContent/**"
+      instructions: |
+        Check tiles against .claude/skills/build-plugin/references/oob-content.md
+        (Dashboard rules, dataSourceConfig and Templating tokens) and the
+        SmartViz notes in REVIEW.md's Platform facts. Flag any {{...}} that
+        is not a templating token and not wrapped in {{{{raw}}}}.
+    - path: ".claude/skills/**"
+      instructions: |
+        These are instructions for AI agents, not plugin files. Do not apply
+        plugin rules (versioning, CODEOWNERS, one plugin per PR) here. Flag
+        guidance that is wrong, contradicts REVIEW.md or another reference, or
+        copies an explanation found in another reference. REVIEW.md restates
+        rules as a short reviewer checklist on purpose; do not flag that
+        overlap. AGENTS.md says which file is authoritative for each topic.
+
+  pre_merge_checks:
+    docstrings:
+      mode: "off" # plugins are JSON plus small scripts; coverage is meaningless here
+    description:
+      mode: "off" # replaced by the template check below
+    custom_checks:
+      - name: "PR template"
+        mode: "warning"
+        instructions: |
+          The PR description must follow one of the templates in
+          .github/PULL_REQUEST_TEMPLATE/: "Add a new plugin", "Change to an
+          existing plugin" or "Miscellaneous change". Pass only if the body
+          contains that template's section headings. For "Add a new plugin"
+          the "Test plan" section, and for "Change to an existing plugin" the
+          "Testing" section, must also have real content. "Miscellaneous
+          change" has no testing section, so do not require one. Extra
+          sections the author adds are fine. "Miscellaneous change" is for
+          PRs that do not add or change a plugin: if files under plugins/
+          change, fail it unless the body declares a repository-wide change
+          (for example, removing one kind of file from every plugin); CI's
+          scope check will still fail such a PR, and a maintainer merges past
+          it. Fail if
+          the body is free text, or is still the template picker from
+          .github/pull_request_template.md.
+
+  tools:
+    # Biome stays on (the default) for its script and duplicate-key checks.
+    # Its "Illegal return statement" false positive on scripts is
+    # suppressed by the plugins/** path instruction above.
+    # SkillSpector stays on too: outside users install the skills in
+    # .claude/skills/, so prompt-injection checks are worth some noise.
+    # LanguageTool's grammar suggestions cut across the REVIEW.md wording
+    # rules.
+    languagetool:
+      enabled: false
+
+knowledge_base:
+  code_guidelines:
+    enabled: true
+    filePatterns:
+      - "REVIEW.md"
+      - "AGENTS.md"
+      - ".claude/skills/build-plugin/references/*.md"
+  learnings:
+    scope: local
+    # Several learnings saved from single PRs contradicted REVIEW.md. Give
+    # maintainers a week to reject a learning before it applies.
+    approval_delay: 7