docs: document fixed_grain and reaggregate - #2589
Merged
Merged
Conversation
Neither declaration was covered for modelers. `fixed_grain` appeared only as a one-line table row in the YAML reference, and `reaggregate` was absent entirely -- the word appears in the internals doc meaning something else (re-aggregating to a coarser grain for windows), which made the gap easy to miss. The share-of-total section was also out of date. It told readers to hardcode a category per metric or divide in their application code, which `fixed_grain: []` now does natively. It is rewritten around the feature, keeping a narrowed pointer to #1695 for the case still unsupported: a numerator and denominator needing different grains from the same fanned fact. Both sections record the behaviour that is hard to guess from the feature names. A fixed grain broadcasts an aggregate but does not deduplicate one, so on a parent that repeats an entity across the dimension being sliced the inner aggregate overcounts and the partition sums the overcount. A reaggregate rule collapses a dimension but does not build a window, so it cannot turn daily flags into a distinct count over a trailing window. Examples use the existing default.sales and snapshot vocabulary. Every behavioural claim, including the emitted SQL shapes, was checked against a running server rather than read off the source.
✅ Deploy Preview for thriving-cassata-78ae72 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The node config examples on the metrics page were API payloads, while nodes are mostly authored as YAML. Converts all six, including the three that predate this branch, so the page is one format throughout rather than split by which section you land on. The nested reaggregate rules in particular read better without the braces and quotes.
Two rules on fixed_grain's dimension list were documented that DJ already raises clear errors for -- one tells you to add the dimension to the query, the other suggests a global grain outright. You meet both at the moment they matter, already solved. What stays is what fails quietly: a fixed grain broadcasting an overcount rather than deduplicating, and a reaggregate rule being mistaken for a window. Neither raises anything; both return a plausible wrong number. Also drops the difficulty framing from the share-of-total opener and a line about list order that carried nothing.
The warning explained the double aggregation in the abstract, which is hard to follow at the moment you need it. Replaced with three rows of data where the wrong answer is visible: two accounts, sliced by device, summing to three. Also repoints the tracking link. #1695 was superseded by #2245, which opens by reframing it and covers off-query-grain metrics generally -- including the case this warning describes.
Three passages in the semi-additive section read as reassurance rather than description -- 'still works as expected', 'behaves like any other metric', 'answer the question correctly instead of refusing it'. A reader wants the result, not confidence that the result is right. Each now states the output: what slicing returns, what a query with the dimension in the grain returns, and what the two options give back when a query omits the dimension.
The heading carried two sentences, one of which only set up the other. Since required_dimensions is documented further down the same page, a reader may wonder why not use that instead -- so the distinction stays, as a sentence linking to it, without a section around it.
The warning said a semi-additive metric cannot be queried alongside a plain additive one from the same parent. Two things wrong with that: the shared parent is irrelevant, and the restriction is conditional. _validate_reaggregate_base_group_join_safety fires only when a query produces multiple grain groups and one of them is semi-additive. Include the collapsed dimension in the output grain and both metrics land in the same group, which compiles fine -- verified against a running server, not inferred from the one query that happened to fail.
The grain-group warning existed to compensate for an unhelpful error message. The message is the thing to fix; a docs entry restating it is not. Removed, and the error text is worth improving separately so it names the remedy. The remaining note was written in vocabulary from the investigation behind these docs -- daily flags, trailing windows, distinct counts -- none of which appears on this page. It now says the thing plainly: a rule picks a value along the dimension, it does not combine values across it.
shangyian
changed the title
fixed_grain and reaggregate
shangyian
marked this pull request as ready for review
September 25, 2026 21:00
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
1 participant
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.
Summary
Add documentation for share-of-total metrics and semi-additive measures.
Test Plan
make checkpassesmake testshows 100% unit test coverageDeployment Plan