Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .claude/skills/metric-e2e-test/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,43 @@ ls specs/*.test.yaml # list existing tests
```

`<name>` is the file stem (e.g. `collab_emails_sent` for `specs/collab_emails_sent.test.yaml`). Warm re-runs are fine — the session resets the multi-reader collab silver/staging tables at start (conftest). `./e2e.sh down` is only the e2e compose teardown (it is not a deploy), for when you want a fully clean ClickHouse.

## New bronze table for a not-yet-seeded connector

The seeder INSERTs into a table that MUST already exist (it reads
`system.columns` and fails otherwise — it does NOT create from the schema YAML).
Bronze tables come from `src/ingestion/scripts/create-bronze-placeholders.sh`
(the rig parses the `run_ch <<'SQL' … SQL` heredocs out of it). So to seed a
connector that isn't there yet:

1. Add `CREATE DATABASE IF NOT EXISTS bronze_<snake>;` to the database heredoc.
2. Add a `CREATE TABLE IF NOT EXISTS bronze_<snake>.<stream> (…)` block (inside a
`run_ch <<'SQL' … SQL` heredoc) with the columns your dbt model reads + the 4
`_airbyte_*` CDK columns. Real Airbyte overwrites it on first sync.
3. Add a matching `schemas/bronze_<snake>.<stream>.yaml` (every column;
`additionalProperties: false`) and a base template covering all of them.

## Gotchas (rig operations + cross-test impact)

- **Stale binary / your migration didn't run.** Historically the biggest trap:
`./e2e.sh` builds analytics-api into the `cargo-target` Docker volume, and on
Docker Desktop (macOS) the mtimes cargo reads through the bind mount don't
reliably advance, so cargo relinked a stale object and the binary silently
lacked new SeaORM migrations (symptoms: `query_ref`/catalog changes have no
effect, a `find` matches 0 rows, `size(items)` off by your new key). FIXED in
`e2e_lib/analytics_api.py::build` — it now `touch`es the analytics-api crate
sources before `cargo build`, forcing a recompile every run (~1-2 min, only
that crate). So a plain `./e2e.sh test` picks up new migrations now; you should
NOT need `down -v` for this. If you still suspect a stale binary, confirm by
querying `seaql_migrations` (below) — your migration version must be present.
- **`1045 Access denied for user 'insight'` at API startup.** Stale
`compose/.env` creds vs a persisted MariaDB volume. Same `down -v` fixes it.
- **Inspect the live DB after a run.** CH + MariaDB stay UP after `./e2e.sh test`
(only the runner is `--rm`). Query directly:
`docker exec insight-e2e-mariadb mariadb -uroot -p"$(grep ^MARIADB_ROOT_PASSWORD compose/.env|cut -d= -f2)" analytics -e "SELECT version FROM seaql_migrations"`
and `docker exec insight-e2e-clickhouse clickhouse-client -q "SELECT … FROM silver.class_<X>"`.
- **Cross-test impact.** Adding a `metric_key` to a shared bullet section raises
that section's `size(items)` for EVERY test that queries it — bump the sibling
tests' count assertions in the same change (e.g. the Zulip add moved the
Collaboration bullet 20 → 21, so `collab_emails_sent.test.yaml` needed the bump
too).
63 changes: 63 additions & 0 deletions cypilot/.core/skills/connector/workflows/create.md
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,69 @@ Rules:
no emails), skip this section and document in the README how identities
resolve instead (cross-connector JOIN, Silver Step 2 direct mapping).

#### 3.6c Surfacing the connector in a dashboard metric (silver class → gold → query_ref → catalog)

> **A connector that only reaches silver does NOT appear in the UI.** Tagging a
> model `silver:class_<X>` makes `union_by_tag` fold it into the shared
> `silver.class_<X>` table — and that is ALL that is automatic. The gold views,
> the metric `query_ref`s, and the metric catalog each enumerate their inputs
> EXPLICITLY; a new source contributes nothing past silver until you wire four
> more layers by hand. Skip this whole section if the connector is bronze-only
> or its silver class is not consumed by any gold metric (then say so in the
> README). Reference implementation: PR constructorfabric/insight#1466 (added
> Zulip chat to the Collaboration bullet) — open it as a worked example.

The data path for a per-person dashboard metric:

```
bronze_<snake>.<stream>
→ <snake>__<class>.sql (silver, tag silver:class_<X>) ← §3.5
→ silver.class_<X> (union_by_tag — AUTOMATIC)
→ insight.<section>_bullet_rows (gold VIEW, CH migration) ← layer A
→ metrics.query_ref (…IC/Team/member/dept) (SeaORM) ← layer B
→ metric_catalog row (label/threshold, SeaORM) ← layer C
→ analytics-api → person-profile <Section> card (UI)
```
Comment on lines +450 to +458

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add a language to the fenced code block.

Line 450 uses an unlabeled fenced block (MD040), which can break docs lint.

Suggested fix
-```
+```text
 bronze_<snake>.<stream>
   → <snake>__<class>.sql            (silver, tag silver:class_<X>)   ← §3.5
     → silver.class_<X>              (union_by_tag — AUTOMATIC)
       → insight.<section>_bullet_rows   (gold VIEW, CH migration)    ← layer A
         → metrics.query_ref (…IC/Team/member/dept)  (SeaORM)         ← layer B
           → metric_catalog row      (label/threshold, SeaORM)        ← layer C
             → analytics-api → person-profile <Section> card (UI)
</details>

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

```suggestion

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 450-450: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cypilot/.core/skills/connector/workflows/create.md` around lines 450 - 458,
The fenced diagram block in the workflow docs is unlabeled, triggering the
markdown lint rule. Update the fenced block around the bronze_<snake> to
analytics-api flow to specify a language tag on the opening fence, and keep the
rest of the content unchanged so the documentation still renders correctly while
satisfying MD040.

Source: Linters/SAST tools


What to add (use an EXISTING section like `collab` / `git` / `ai` / `support`
when the metric fits one; only create a new `<section>_bullet_rows` view +
metric ids for a genuinely new section):

- **Layer 0 — silver class build edge.** Add
`-- depends_on: {{ ref('<snake>__<class>') }}` to
`src/ingestion/silver/<domain>/class_<X>.sql`. `union_by_tag` already includes
your model at SQL level once its table exists; this edge makes a
`tag:<slug>+` prod run rebuild the class (and the e2e rig's `derive_selectors`
build it from the `silver:` tag).
- **Layer A — gold view branch.** In the CH migration that defines
`insight.<section>_bullet_rows`
(`src/ingestion/scripts/migrations/*-bullet-rewrite.sql`; idempotent
`DROP+CREATE`, no tracking table → edit the canonical definition in place),
add a branch `... FROM silver.class_<X> WHERE data_source = 'insight_<snake>'`
emitting your `metric_key`(s). The `data_source` literal is exactly what your
silver model SELECTs (`'insight_<snake>'`). Join `insight.people` on
`lower(email) = p.person_id` for `org_unit_id`.
- **Layer B — query_ref(s).** The bullet `query_ref`s materialize EVERY
FE-visible `metric_key` as `sumIf(metric_value, metric_key='<k>') AS <k>_v`
then unpivot via `ARRAY JOIN [('<k>', <k>_v), …]`. Add your key to BOTH lists.
SeaORM migrations are append-only — write a NEW
`m<date>_<n>_<section>_<conn>.rs` that `UPDATE metrics SET query_ref=… WHERE
id = UNHEX('<hex>')`, basing the SQL on the LATEST migration that set that
query_ref (grep the metric id; a section has SEVERAL copies — IC bullet,
Team bullet, member-values `…0041`, dept-distribution `…0045` — update each
surface you need). Register the migration in
`src/backend/services/analytics-api/src/migration/mod.rs`.
- **Layer C — catalog row.** New append-only migration
`m<date>_<n>_seed_<conn>_<section>_catalog.rs` inserting a `metric_catalog`
row (`<section>_bullet_rows.<metric_key>`, label, sublabel, unit,
`source_tags: ["<slug>"]`) + a product-default `metric_threshold`. Model it on
`m20260620_000002_seed_wiki_catalog.rs`. Register in `mod.rs`.

Then prove the whole chain with a `/metric-e2e-test` fixture (see that skill) —
seed bronze, query the bullet metric, assert your `metric_key`'s value. Adding a
key to a shared section raises its `size(items)`, so bump any sibling test that
asserts the old count.

### For CDK (`CONNECTOR_TYPE=cdk`):

Create Python scaffold:
Expand Down
15 changes: 15 additions & 0 deletions cypilot/.core/skills/connector/workflows/validate.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,21 @@ Exit 0 = PASS for the targeted connector(s); exit 2 = at least one FAIL. Rule ID
| `BP-8` | every `promote_bronze_to_rmt` call passes `order_by` |
| `BP-9` | every other model that reads bronze depends on `<name>__bronze_promoted` |

### Dashboard metric surfacing (only if the connector should appear in the UI)

A connector whose silver class feeds a dashboard metric does NOT surface in the
UI from the silver model alone — the gold view, the metric `query_ref`(s), and
the catalog each enumerate inputs explicitly (see `connector-create.md` §3.6c).
If the connector is expected to show a per-person card, verify the full chain;
if it is bronze-only or its class has no gold consumer, confirm the README says
so and skip this section.

- [ ] Silver class `class_<X>.sql` carries `-- depends_on: {{ ref('<snake>__<class>') }}` for the connector.
- [ ] The gold `insight.<section>_bullet_rows` view has a branch `FROM silver.class_<X> WHERE data_source = 'insight_<snake>'` emitting the connector's `metric_key`(s). (`data_source` literal == what the silver model SELECTs.)
- [ ] EVERY bullet `query_ref` that should show the key was re-set in a NEW append-only SeaORM migration — the key appears in both the `sumIf(... metric_key='<k>') AS <k>_v` list AND the `ARRAY JOIN [('<k>', <k>_v), …]` unpivot. A section typically has several copies (IC `…0012`-style, Team, member-values `…0041`, dept-dist `…0045`); each must be updated for the surface it backs. Base the new SQL on the LATEST migration that set that id (grep the metric hex id), and register the migration in `migration/mod.rs`.
- [ ] A `metric_catalog` row + product-default `metric_threshold` for `<section>_bullet_rows.<metric_key>` exists (new append-only migration, `source_tags: ["<slug>"]`, registered in `mod.rs`).
- [ ] An e2e fixture (`/metric-e2e-test`) seeds bronze and asserts the metric_key's value end-to-end; any sibling test asserting the section's `size(items)` was bumped for the new key.

### Credentials Template
- [ ] `credentials.yaml.example` lists all required fields
- [ ] `insight_source_id` is included
Expand Down
Loading
Loading