metric-create: new skill for fact tables and fact metrics - #16
Open
Berenger-Wooclap wants to merge 1 commit into
Open
metric-create: new skill for fact tables and fact metrics#16Berenger-Wooclap wants to merge 1 commit into
Berenger-Wooclap wants to merge 1 commit into
Conversation
Closes the gap metric-search points at: creating a metric meant leaving for the UI. Covers all six metric types, row filters, aggregate filters, and quantile settings, plus creating the fact table underneath when one doesn't exist yet. Fourth write skill. Scope stops at the metric definition. Analysis settings (windows, capping, priors, regression adjustment, risk thresholds, sample size, MDE) inherit the org defaults and are tuned in the UI, so the skill doesn't pretend to own them. Guardrails were verified against the handlers and validators rather than the OpenAPI descriptions, which turned out to disagree in three places: - postFactMetric overwrites numerator.column for proportion, retention, and dailyParticipation (to $$distinctUsers / $$distinctDates) instead of rejecting a supplied column, and clears aggregation for those types - a ratio metric with no denominator is accepted, not rejected, and produces a metric that can't compute - quantileSettings is documented as mandatory for quantile metrics but is optional in the schema Also documents that fact table columns are detected by a queued job (so the create response is empty), and that userIdTypes is validated against the datasource's configured identifier types. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opening this at @jdorn's suggestion on growthbook/growthbook-mcp#66 — that PR added
create_fact_table/create_fact_metricas hand-written MCP tools, which #63 made the wrong shape. This is the same competence, remapped to a skill.What it adds
metric-create— create a fact metric, and the fact table underneath it when one doesn't exist yet. Fourth write skill, afterflag-create,experiment-launch, andexperiment-stop.It fills the gap
metric-searchcurrently points at: its read-only guardrail said to route metric creation to the GrowthBook UI. Now it routes here.Covers all six metric types, row filters, aggregate filters, quantile settings, and the fact-table path (data source resolution → SQL → async column detection → metric).
Scope stops at the metric definition. Conversion windows, capping, priors, regression adjustment, risk thresholds, sample size, and MDE inherit the org defaults and are tuned in the UI. A ~25-field request body isn't something an agent fills in well, and those fields are exactly the ones where a wrong guess is invisible until the experiment reads wrong.
Guardrails came from the handlers, not the spec
Following the top-of-file rule in
CLAUDE.md, every guardrail was checked againstpackages/back-end/src/api/fact-{metrics,tables}/post*.tsandpackages/shared/src/validators/fact-{metrics,table}.ts. Three of them disagree with the OpenAPI descriptions, in the direction that matters — the docs describe constraints the API doesn't enforce:numerator.column"must be empty for proportion and dailyParticipation"getCreateMetricPropsFromBodyoverwrites it —$$distinctUsersforproportionandretention,$$distinctDatesfordailyParticipation— and clearsaggregationfor all three. A supplied column is silently discarded, not rejected.denominator"only when metricType is 'ratio'"quantileSettings"mandatory if metricType is quantile".optional()in the schema. Omitting it yieldsquantileSettings: nullrather than a 400.I mention it because I originally implemented these as blocking validation in the MCP PR — which would have rejected requests GrowthBook accepts. Reading the handlers is what caught it. If you'd rather the descriptions moved toward the behavior, that's a separate change in the main repo and I'm happy to open it.
Two more worth having written down, also from the handlers:
POST /fact-tablesqueuesrefreshFactTableColumns, so the create response'scolumns[]is empty. The skill re-fetches before defining a metric rather than guessing column names.userIdTypesis validated againstdatasource.settings.userIdTypesand fails withInvalid userIdType: <value>. The skill reads them fromGET /api/v1/data-sourcesfirst instead of assuminguser_id.Files kept in sync
skills/metric-create/SKILL.md+ thescripts/gb-callsymlinkskills/metric-search— creation now routes here; added to HandoffsREADME.md— Product analytics table, the chaining list, the file tree, and the "What these skills do not do" bullet (narrowed to datasource creation and analysis tuning)CHANGELOG.md— under[Unreleased]Structure follows the contract in
CLAUDE.md: frontmatter with routing-oriented description,allowed-toolspinned togb-calland nothing else, numbered workflow with literal bash and JSON, Guardrails, Endpoints used, Handoffs.Happy to split the fact-table path into its own skill if you'd rather keep one responsibility per skill — I kept them together because creating a fact table is almost never the user's actual goal, it's a prerequisite they hit on the way to a metric.
🤖 Generated with Claude Code