Skip to content

Commit b4e0958

Browse files
authored
Enforce that descriptions live in doc blocks, and document the process (#325)
* First pass at restructuring .md doc files * Add hooks to pre-commit to enforce .md rules * Update documentation and claude.md for docs practices * Cleanup * Trim comments * Trim claude.md * Copilot suggestions * address copilot review comments * remove hook exlusion list * Remove description changes * Update linter, per copilot comments * Pass linter, add doc blcoks for toke_balance
1 parent 7bfb6f1 commit b4e0958

39 files changed

Lines changed: 1170 additions & 274 deletions

.claude/CLAUDE.md

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -120,26 +120,14 @@ See `docs/snapshot.md` for the full control flow diagram. For hands-on snapshot
120120

121121
## Documentation
122122

123-
- **Universal column definitions**: `models/docs/universal.md` — referenced via `{{ doc('column_name') }}` in YAML schema files
124-
- **Domain docs**: `models/docs/sources/`, `models/docs/snapshots/`, `models/docs/marts/`, `models/docs/intermediate/`
125-
- Each SQL model has a co-located `.yml` schema file with column descriptions and tests
126-
127-
When adding or modifying models, update both the co-located YAML and any relevant doc blocks in `models/docs/`.
123+
- **Model or column descriptions:** read `docs/documentation.md` before writing one.
128124

129125
## Pre-commit Hooks
130126

131-
Pre-commit runs automatically on commit. Run it manually before finishing any task:
127+
Run before considering a task complete. See `.pre-commit-config.yaml` for the hook list.
132128

133129
```bash
134-
pre-commit run --all-files # Run all hooks on all files
135-
pre-commit run --files path/to/file.sql # Run on specific files
130+
pre-commit run --all-files
136131
```
137132

138-
Hooks:
139-
1. **SQLFluff** — lints and auto-fixes SQL style
140-
2. **dbt-checkpoint** — enforces:
141-
- All model columns in `marts/` must have descriptions in `.yml`
142-
- All mart models must have a description
143-
- Model tags must be from the approved allowlist (see `.pre-commit-config.yaml`)
144-
- All source columns/tables must have descriptions
145-
3. **Prettier** — formats `.json`/`.yaml`/`.yml` files
133+
`--files <list>` can report "no files to check" in some environments; `--all-files` is reliable.

.github/pull_request_template.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ change is, and why it is being made, with enough context for anyone to understan
1616

1717
- [ ] This PR adds tests for the most critical parts of the new functionality or fixes.
1818
- [ ] I've updated the docs and README with the added features, breaking changes, new instructions on how to use the repository.
19+
- [ ] If this PR touches column or model descriptions, they live in doc blocks under `models/docs/` rather than inline in the `.yml`, and I read the text of every block I referenced rather than trusting its name (see [docs/documentation.md](../docs/documentation.md)).
1920

2021
### Release planning
2122

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,7 @@ credentials.json
2020
### dbt generated files
2121
package-lock.yml
2222
.user.yml
23+
24+
# Python bytecode from scripts/
25+
__pycache__/
26+
*.py[cod]

.pre-commit-config.yaml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,18 @@ repos:
4444

4545
- repo: local
4646
hooks:
47+
- id: docs-lint
48+
name: Descriptions live in doc blocks, not inline in yml
49+
entry: python scripts/docs_lint.py check
50+
language: python
51+
additional_dependencies: ["pyyaml"]
52+
pass_filenames: false
53+
# The check reads the whole repo and takes well under a second, so it runs
54+
# unconditionally. A `files` filter would skip it on a commit that only
55+
# deletes a referenced .md, which is exactly when a dangling doc()
56+
# reference appears.
57+
always_run: true
58+
4759
- id: cleanup
4860
name: Cleanup .env.tmp
4961
entry: ./pre-commit/cleanup.sh

README.md

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ If branch is already made, just rename it _before passing the pull request_.
2020
- [dbt Project Structure](#dbt-project-structure)
2121
- [Development Folders](#development-folders)
2222
- [Tests](#tests)
23+
- [Documentation](#documentation)
2324
- [Getting Started](#getting-started)
2425
- [Download the Repo](#download-the-repo)
2526
- [Configure dbt](#configure-dbt)
@@ -96,7 +97,7 @@ What not to do in intermediate:
9697
> _*Note:*_ More information about `intermediate` layer can be found [here](https://docs.getdbt.com/best-practices/how-we-structure/3-intermediate).
9798
9899
3. Marts
99-
The marts layer is where users can access final dimensional modeling tables. Each model will be accompanied by a `.yml` file with the same name. The `.yml` file contains the descriptions and tests for the model and its columns.
100+
The marts layer is where users can access final dimensional modeling tables. Each model will be accompanied by a `.yml` file with the same name, holding the tests for the model and its columns plus a `{{ doc(...) }}` reference for each description. The description text itself lives in a doc block under `models/docs/`, never inline in the `.yml`. See [Documentation](#documentation).
100101

101102
What to do in marts:
102103

@@ -141,7 +142,7 @@ What not to do in snapshots:
141142
| Staging | Stages the pre-processed or cleaned data before performing the transformations. |
142143
| Intermediate | Contains the transformed and processed data. |
143144
| Marts | Houses the final data models or data marts, which are the end results of the dbt project. |
144-
| Docs | Stores documentation related to your dbt project. |
145+
| Docs | Holds the reusable column and model descriptions (doc blocks), mirroring the `models/` layout. |
145146
| Macros | Contains reusable SQL code snippets known as macros. |
146147
| Tests | Contains defining tests to validate the accuracy and correctness of the data transformations. |
147148

@@ -161,6 +162,40 @@ There are three different test types:
161162
162163
<br>
163164

165+
## Documentation
166+
167+
Every model, source table and column carries a description, and those descriptions are published to the dbt docs site on every merge to `master`. There is one rule:
168+
169+
**Descriptions live in doc blocks, never inline in the `.yml`.**
170+
171+
The `.yml` holds a reference; the text lives in a `.md` file under `models/docs/`:
172+
173+
```yaml
174+
- name: asset_code
175+
description: '{{ doc("asset_code") }}'
176+
```
177+
178+
```markdown
179+
{% docs asset_code %}
180+
The 4 or 12 character code representation of the asset on the network.
181+
{% enddocs %}
182+
```
183+
184+
That gives each description one home, so when the same column appears in a source, a staging model and a mart, all three point at the same block and the text cannot drift between them.
185+
186+
`models/docs/` mirrors `models/`, so a column on `models/marts/trade_agg.sql` is documented in `models/docs/marts/trade_agg.md`. A definition that **unrelated tables** share, like `asset_code` or `batch_run_date`, goes in `models/docs/universal.md` instead. A column that merely flows from `sources/` through `staging/` to `marts/` is still one table's column and stays in that table's mirror file.
187+
188+
Before writing a description, check whether a block already exists, and read its text rather than trusting its name:
189+
190+
```bash
191+
grep -rn "{% docs <column_name> %}" models/docs/
192+
./venv/bin/python scripts/docs_lint.py check # also runs in pre-commit
193+
```
194+
195+
> _*Note:*_ The full process, including doc block naming, what the linter deliberately does not check, how to prove a docs change rendered nothing unexpected, and what to check before renaming a block that `stellar-dbt` depends on, is in [docs/documentation.md](./docs/documentation.md)
196+
197+
<br>
198+
164199

165200
# Getting Started
166201

docs/documentation.md

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
# Documenting models and columns
2+
3+
Every model, source table and column carries a description, published to the dbt docs site on
4+
every merge to `master` (`.github/workflows/dbt-docs-website.yml`).
5+
6+
- [The rule](#the-rule)
7+
- [Where a block lives, and what to call it](#where-a-block-lives-and-what-to-call-it)
8+
- [Adding a column](#adding-a-column)
9+
- [Running the linter](#running-the-linter)
10+
- [What the linter does not catch](#what-the-linter-does-not-catch)
11+
- [Proving a change is docs-neutral](#proving-a-change-is-docs-neutral)
12+
- [File conventions](#file-conventions)
13+
- [Relationship to stellar-dbt](#relationship-to-stellar-dbt)
14+
15+
## The rule
16+
17+
**Descriptions live in doc blocks, never inline in yml.** The yml holds a reference:
18+
19+
```yaml
20+
- name: asset_code
21+
description: '{{ doc("asset_code") }}'
22+
```
23+
24+
and the text lives in a `.md` under `models/docs/`:
25+
26+
```markdown
27+
{% docs asset_code %}
28+
The 4 or 12 character code representation of the asset on the network.
29+
{% enddocs %}
30+
```
31+
32+
No allowlist, no per-description exception, no way to defer a path, no threshold to tune.
33+
`scripts/docs_lint.py check` enforces it and runs in pre-commit.
34+
35+
## Where a block lives, and what to call it
36+
37+
`models/docs/` mirrors `models/`: a column on `models/marts/trade_agg.sql` is documented in
38+
`models/docs/marts/trade_agg.md`. Create the mirror file if it does not exist.
39+
40+
A definition that **unrelated tables** share goes in `models/docs/universal.md` instead, named
41+
after the bare column (`asset_code`, `batch_run_date`, `closed_at`). A column that merely flows
42+
`sources/` -> `staging/` -> `marts/` touches several files but is still one table's column, so
43+
it stays in that table's mirror file.
44+
45+
A definition specific to one model is named `<model>__<column>`, for example
46+
`int_tvl_trustlines__asset_code` because that column carries non-XLM TVL only. Use the scoped
47+
form whenever a column's meaning differs from the shared block of the same name, even slightly.
48+
`amount_raw` (i128 base units) and `amount` (decimals applied) are different concepts and get
49+
different blocks.
50+
51+
Block names are globally unique across the project and resolve by name, not by file, so moving
52+
a block between files is always safe.
53+
54+
`check` enforces the root: a definition this repo owns must live somewhere under `models/docs/`.
55+
Which file it lands in inside that root is convention, not enforced, so the mirror rule above is
56+
guidance rather than a gate. `docs_lint.py report` shows how many tables use each block if you
57+
want to check whether something has outgrown its home.
58+
59+
## Adding a column
60+
61+
1. Add the column to the model's `.yml` alongside its tests.
62+
2. Look for an existing block: `grep -rn "{% docs <column_name> %}" models/docs/`
63+
3. If one exists, read its text and confirm it describes your column. If it does not, write a
64+
new block named `<model>__<column>`.
65+
4. Otherwise write a block in the model's mirror `.md`.
66+
5. Run `docs_lint.py check`.
67+
68+
## Running the linter
69+
70+
No warehouse connection or credentials needed; it reads the yml and md files directly.
71+
72+
```bash
73+
./venv/bin/python scripts/docs_lint.py check # the gate; also runs in pre-commit
74+
./venv/bin/python scripts/docs_lint.py report # diagnostics, never fails a build
75+
```
76+
77+
`check` requires that the whole value of a `description:` is one `doc()` call naming a block that
78+
exists, so it fails on inline text, on text wrapped around a reference
79+
(`see {{ doc("x") }} for detail`), on an empty description, on two references in one value, and
80+
on a name that is not defined anywhere. It also fails on a yml that will not parse (the rule
81+
cannot be applied to a file that cannot be read) on two files defining the same block name
82+
(dbt cannot resolve a duplicate), and on a definition this repo owns that sits outside
83+
`models/docs/`, which is how a definition drifts somewhere nobody thinks to look.
84+
85+
**Macro and argument descriptions are out of scope.** They document one macro's signature, so
86+
there is nothing to factor out, and moving them into `models/docs/` would only put a macro's API
87+
docs further from the macro.
88+
89+
`report` lists orphan blocks, how many tables use each block, columns declared twice under one
90+
resource, and any column name that resolves to more than one description.
91+
92+
## What the linter does not catch
93+
94+
**A `doc()` that points at the wrong block.** The rule checks that a description *is* a
95+
reference, not that the reference is *correct*, so both of these pass:
96+
97+
```yaml
98+
- name: asset_b_type
99+
description: '{{ doc("asset_a_type") }}' # renders "the sold asset", not "the bought asset"
100+
- name: operation_id
101+
description: '{{ doc("transaction_id") }}' # renders "a unique identifier for this transaction"
102+
```
103+
104+
Both of those are live in this repo right now, along with nineteen more. They come from copying
105+
a neighbouring line and cluster on paired columns: `asset_a` / `asset_b`, `read_bytes` /
106+
`write_bytes`, `batch_id` / `batch_run_date`. Repairing them changes published text, so it is
107+
tracked separately in #326 and deliberately not part of the change that added this linter.
108+
109+
This is a code review responsibility. When reviewing a description change, read the block that
110+
is referenced rather than trusting its name. The last section of `docs_lint.py report` helps: a
111+
column that disagrees with itself inside one table family is nearly always one of these.
112+
113+
**The same column declared twice in one yml.** yaml keeps the last entry, so the first
114+
description is dropped and the column publishes the second one's text. dbt does not warn.
115+
`report` lists these; five remain, and `enriched_history_operations_soroban.memo_type` was one
116+
of them, which is why `memo` had no description at all.
117+
118+
## Proving a change is docs-neutral
119+
120+
Moving blocks between files should not change a single rendered description. Prove it:
121+
122+
```bash
123+
git stash push --include-untracked # -u matters: a new untracked .yml or .md
124+
./venv/bin/python scripts/docs_lint.py snapshot --out /tmp/before.json
125+
git stash pop
126+
./venv/bin/python scripts/docs_lint.py snapshot --out /tmp/after.json
127+
./venv/bin/python scripts/docs_lint.py diff /tmp/before.json /tmp/after.json
128+
```
129+
130+
A pure relocation must print `0 differences`; a text change must print exactly what you meant
131+
to change. Paste that output into the PR: it is the cheapest way to show a reviewer that a large
132+
diff is safe.
133+
134+
The snapshot is trustworthy because the resolver is checked against dbt itself with
135+
`docs_lint.py validate-manifest`, which needs a `target/manifest.json` from `dbt parse` or
136+
`dbt docs generate`. It exits non-zero unless `MISMATCHED` is 0, nothing is unresolved, and every
137+
description it could not compare is one an installed package patched in, which this repo cannot
138+
see by design.
139+
140+
One thing that looks like it should work and does not: **a `doc()` call inside a doc block.** dbt
141+
renders block bodies without the `doc` macro in scope and fails the parse with
142+
`'doc' is undefined`, so a shared definition cannot be wrapped with a local qualifier. Write a
143+
scoped `<model>__<column>` block with the full text instead.
144+
## File conventions
145+
146+
- One `.md` per model or source table, mirroring the model's path. Where sibling models each
147+
need only a model-level description, one file for the group is fine (see
148+
`models/docs/intermediate/trades/int_trade_agg.md`).
149+
- Open each file with a subject comment: `[comment]: < Trade Aggregations -`. Most files use
150+
exactly that form, dangling hyphen included; prefer it in new files.
151+
- Split `universal.md` into subject files (`asset.md`, `ledger_state.md`, `batch.md`) at around
152+
60 to 80 blocks. Below that, one file is easier to search.
153+
- `models/docs/sources/history_operations.md` holds 125 blocks and is the one file that is
154+
genuinely too large. Splitting it is tracked separately.
155+
156+
## Relationship to stellar-dbt
157+
158+
`stellar-dbt` installs this repo as a git package and references roughly 200 doc blocks defined
159+
here. **Never rename or delete a block** without checking there first: a rename breaks its parse
160+
and costs a `packages.yml` pin bump plus `dbt deps` to unwind. Moving a block between files is
161+
safe.
162+
163+
To check before merging something risky, point `stellar-dbt`'s `packages.yml` at your branch and
164+
run `dbt deps && dbt parse` there. A clean parse confirms every cross-repo reference resolves.
165+
166+
`scripts/docs_lint.py` is meant to be identical in both repos: it reads
167+
the project name from `dbt_project.yml` rather than hardcoding it, so a change made here is
168+
copied across verbatim rather than ported by hand.

models/docs/intermediate/int_asset_metadata.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,11 @@ The full, human-readable name for the token (e.g., "Circle USD").
2121
This is part of the standard metadata required for SEP-41 compliance. Consistent naming ensures that the token is handled correctly by explorers and interoperable contracts built to support Soroban's built-in tokens.
2222

2323
{% enddocs %}
24+
25+
{% docs int_asset_metadata %}
26+
One row per contract_id with the contract's asset_code (coalesced from stg_assets SAC asset_code, then SEP-41 symbol read from contract storage) plus the SEP-41 metadata fields (symbol, name, decimal, admin) read directly from contract instance storage. asset_code is null when neither a SAC asset_code nor a SEP-41 symbol is available — recognized assets are enriched upstream in stg_assets, so a null here means the contract publishes no metadata.
27+
{% enddocs %}
28+
29+
{% docs int_asset_metadata__asset_code_source %}
30+
Indicates which source the asset_code came from: 'sac' = asset_code from a Stellar Asset Contract token transfer event, 'metadata' = SEP-41 symbol read from contract storage. Null when no asset_code could be resolved (asset_code is also null in that case).
31+
{% enddocs %}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[comment]: < Token Transfer Enrichment -
2+
3+
{% docs int_token_transfer_enrichment %}
4+
This table joins token_transfers_raw with history_operations to add operation type and soroban flag.
5+
{% enddocs %}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
[comment]: < Trade Aggregation Intermediates -
2+
3+
{% docs int_trade_agg_day %}
4+
Intermediate for the aggregation of daily trade data. This table contains the aggregated metrics of all trades between an asset pair, regardless of whether they are selling or buying.
5+
{% enddocs %}
6+
7+
{% docs int_trade_agg_month %}
8+
Intermediate for the aggregation of monthly trade data. This table contains the aggregated metrics of all trades between an asset pair, regardless of whether they are selling or buying.
9+
{% enddocs %}
10+
11+
{% docs int_trade_agg_week %}
12+
Intermediate for the aggregation of weekly trade data. This table contains the aggregated metrics of all trades between an asset pair, regardless of whether they are selling or buying.
13+
{% enddocs %}
14+
15+
{% docs int_trade_agg_year %}
16+
Intermediate for the aggregation of yearly trade data. This table contains the aggregated metrics of all trades between an asset pair, regardless of whether they are selling or buying.
17+
{% enddocs %}
18+
19+
{% docs int_trade_agg__day_agg %}
20+
21+
Date from which all metrics are aggregated.
22+
23+
{% enddocs %}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
[comment]: < TVL Intermediates -
2+
3+
{% docs int_tvl_accounts %}
4+
This table aggregates XLM selling liabilities from accounts to calculate TVL
5+
{% enddocs %}
6+
7+
{% docs int_tvl_trustlines %}
8+
This table aggregates asset selling liabilities from trustlines to calculate TVL
9+
{% enddocs %}
10+
11+
{% docs int_tvl_trustlines__account_id %}
12+
The account id for the tvl
13+
{% enddocs %}
14+
15+
{% docs int_tvl_trustlines__asset_code %}
16+
The asset code of the TVL, this table only has non XLM tvl.
17+
{% enddocs %}
18+
19+
{% docs int_tvl_trustlines__asset_issuer %}
20+
The asset issuer of the TVL, this table only has non XLM tvl.
21+
{% enddocs %}

0 commit comments

Comments
 (0)