| name | review-vocabulary |
| description | Review and rewrite the prose a branch adds — docstrings, comments, user-facing messages, docs — against dlt's fixed vocabulary and Simplified Technical English. Invoke as /review-vocabulary. |
| argument-hint | [<branch-or-base-ref>] [-- <extra focus or scope notes>] |
| disable-model-invocation | true |
Review Vocabulary
Rewrite the prose a branch adds so dlt says one thing one way. Two inputs govern every
decision: the fixed vocabulary in this file, and the Simplified Technical English
rules in references/simple-english.md.
Never invoke this on your own. A maintainer runs it with /review-vocabulary. It rewrites
text across dozens of files and changes CI test ids, so it is not a background cleanup.
Parse $ARGUMENTS:
- Everything before the first
-- is the base ref to diff against. Defaults to
$(git merge-base origin/devel HEAD).
- Everything after the first
-- is extra focus from the maintainer.
Read first
references/simple-english.md — the 53 rules. Cite only rule numbers that exist in that
file. The numbering is unintuitive and models invent it. Rule 3.1 is about verb forms, not
sentence length.
references/simple-english-use-cases.md — needed for the error-message shape.
- The fixed vocabulary below. It overrides the general rules where they disagree.
Scope
Diff with git diff <base> — not git diff <base>...HEAD, which misses uncommitted work
that will ship in the same PR.
In scope, when the branch added or changed the line:
| Kind | Where |
|---|
| Docstrings | dlt/, tests/ |
# comments | dlt/, tests/ |
| User-facing messages | raise X("..."), logger.*, warnings.warn, helpers that build a message |
| Test function names | tests/ — renames only under the narrow gate below |
| Documentation | docs/website/docs/** |
| Snippet files a page pulls from | *snippets.py — comments as docs, code as a test (see below) |
When the maintainer asks for a whole-file docs review, pre-existing prose in those files is in
scope too. Say which mode you are in; the default is added-or-changed lines only.
Out of scope:
- Pre-existing prose, in the default mode. Leave it even when it breaks every rule. List what
you skipped and why.
- Code, identifiers, signatures, type annotations — except snippet code, which gets a separate
review against the repo's test rules.
- SQL keywords and anything already in backticks, CLI commands, file paths, config keys.
pytest.param(id=...) strings, fixture names, parameter names.
- Assertions on exact generated SQL.
- Prose moved verbatim from another file — check the base ref before claiming it is new.
Classification
Get this right first; every other rule depends on it.
| Text | Mode | Limit | Shape |
|---|
Docstrings, # comments | descriptive | 25 words (Rule 6.3) | no imperative, one new fact per sentence (Rule 6.1) |
| Exception strings, warnings, log lines | procedural | 20 words (Rule 5.1) | what happened (simple past), the cause, then the fix as an imperative |
| Docs — how-to, steps, install | procedural | 20 words | imperative, condition before command (Rule 5.4) |
| Docs — concept, reference, architecture | descriptive | 25 words | no imperative |
Messages keep their remedies. Many errors end in an instruction — "Materialize the dataset…",
"Set a permanent pipeline_salt…". Improve the wording: make it imperative, condition first,
under 20 words, one instruction per sentence. Never delete a remedy. Never invent one where
the message has none — a remedy needs facts the string does not interpolate.
Count the assembled sentence, not the fragment. Some messages are built by a helper that wraps
a short reason in a frame — _no_data_location(reason) in
dlt/common/destination/client.py is the current example. Read the helper first, then judge each
reason as the clause it becomes inside the finished sentence. A four-word reason can push the
assembled sentence past 20 words.
Docs carry their own untouchables: fenced code blocks, <!--@@@DLT_SNIPPET ...--> markers,
front-matter, link targets and anchor slugs.
Headings are anchors. Renaming one breaks every link to it. Before you rename, grep the whole
docs tree for the old slug and update each hit in the same change. When a rename would need an edit
in a file another agent owns, hand it back rather than reaching across.
Snippet files are BOTH tests and documentation
A page that uses <!--@@@DLT_SNIPPET ./x_snippets.py::name--> pulls real code from a real test.
docs/pyproject.toml collects *snippets.py with *_snippet functions, so docs CI executes
them. They therefore get two reviews at once, against two different rule sets:
| Part of the file | Treat as | Rules |
|---|
| the code | a test | .claude/rules/testing.md, coding-style.md, imports.md |
the # comments | user-facing docs | this vocabulary + STE, classified per comment |
Consequences worth knowing:
- Comments inside a snippet render to the reader, so a stale or wrong one is a doc bug, not a
code-hygiene nit. Judge them as prose. A comment that labels a step is INSTRUCTION; a comment
that explains behavior is REFERENCE.
- The repo comment rule still applies to the code. Default to no comment. But a snippet is
teaching material, so a comment that would be redundant in library code can earn its place here.
Say which of the two you applied when they conflict.
- Never change snippet code to satisfy a prose rule. Changing code changes what CI runs. If the
code is wrong, report it as a code finding with its own justification.
- A fenced code block is NOT a snippet. It is untested. When a page mixes the two, its fenced
examples can drift from the API while its snippets cannot. Flag the untested ones as unverified
rather than assuming they work — and consider recommending they become snippets.
The fixed vocabulary
One word, one meaning, one part of speech (Rules 1.11, 9.4). Apply the tables before any other
rewriting.
The vocabulary is organised in groups, one per area of dlt. A group is self-contained: its
included terms, its excluded terms, and the rulings that are easy to get wrong in that area. Groups
grow independently — a review only needs the groups its diff touches, and adding an area means
adding a group, not editing the others.
Groups defined so far:
| Group | Covers |
|---|
| G1 — Data access and locations | destinations, datasets, physical locations, join compatibility |
| G2 — Attach and foreign datasets | cross-destination joins, attach info, catalog aliases |
| G3 — Transformations and materialization | relations, transformations, model jobs, eager and lazy paths |
| G4 — Identifiers and SQL generation | naming conventions, case-folding, query binding |
| G5 — Configuration and credentials | configs, credentials, secrets |
Two rules apply across every group:
dlt is the sentence subject in messages. That is how an error gets active voice with a
named agent (Rule 3.6): "dlt cannot join…", "dlt cannot determine…". House style.
- A banned word is banned for one meaning, not always. Every group below names its legal
exceptions. Check them before "fixing" a hit.
G1 — Data access and locations
Included
| Concept | Write |
|---|
| getting to data (verb) | access |
| the adjective | accessible, inaccessible |
| the negative, as a verb | cannot access |
| where the data physically sits | data location |
Excluded
| Never | Because |
|---|
| reach, reaches, reachable, in reach of, out of reach, get to | one word for one concept — access |
| physical location, physical destination, physical dataset (in prose) | say data location; the method data_location() keeps its name |
Rulings
access is a VERB. Do not introduce noun uses — no "data access", no "access is one-way".
Write "the engine accesses the data", "only one direction accesses the data". Pre-existing noun
uses stay.
reach is not always access. "SET SESSION would not reach the cloned sessions" means
propagate to, not read data from. A literal swap changes the meaning — restructure
(Rule 9.1).
G2 — Attach and foreign datasets
Included
| Concept | Write |
|---|
the TAttachInfo object | attach info |
the TAttachStatement object | attach statement |
| the SQL keyword | `ATTACH` in backticks |
| the action in prose | attach (lowercase, a verb) |
| the catalog a foreign dataset lands under | attach alias |
Excluded
| Never | Because |
|---|
descriptor (for TAttachInfo) | attach info matches the type and the method _attach_infos() |
bare ATTACH as a prose noun, ATTACHed, "attaches" as a plural noun | backtick the keyword, or use the verb |
| attach instructions | one name — attach statements |
Rulings
descriptor is legal for the Python descriptor protocol. dlt/common/utils.py describes a
real Python descriptor. The ban covers naming the TAttachInfo object only.
attach info and attach statement are different things. One is the whole descriptor for a
foreign dataset; the other is a single SQL statement inside it. Do not collapse them.
G3 — Transformations and materialization
Included
| Concept | Write |
|---|
| the deferred path | lazy materialization |
| the load job that runs it | model job |
| the immediate path | eager materialization |
Excluded
| Never | Because |
|---|
| model extraction | not a thing — it is a model job |
| executed here | say eager materialization |
Rulings
lazy and eager are legal only for materialization. dlt has both. Using lazily to mean
on first use (memoization) is a second meaning for one word — write "on the first read".
- A model job is the artifact; lazy materialization is the path. Use the one you mean.
G4 — Identifiers and SQL generation
Included
| Concept | Write |
|---|
| identifier case handling | case-fold, case-folds, case-folding |
Excluded
| Never | Because |
|---|
| casefold, post-fold (in prose) | one spelling — case-fold; the identifier casefold_identifier keeps its name |
Rulings
- Hyphenated compounds on the
fold root are legal — "foreign-folded output column" reads
correctly and is in use. The ban is on the bare spelling casefold in prose.
G5 — Configuration and credentials
Included
| Concept | Write |
|---|
| a resolved settings object | config |
Excluded
| Never | Because |
|---|
| configuration (for the object) | config; keep "configuration error" when it names ConfigurationValueError |
Legal technical nouns — never replace, any group (Rules 1.5, 1.8)
attach, attach alias, attach info, attach statement, catalog, config, data location, dataset,
destination, duckdb, iceberg, materialization, model job, pipeline, relation, scanner, vended.
Rules this codebase breaks most
The repo comment rule overrides every row. Default to no comment. Keep at most one short line for
a non-obvious WHY. Prefer a rewrite that shrinks the line count. When a comment only restates the
code, delete it.
Test function names
A name is an identifier. Rename it for one of two reasons only:
- it contradicts the fixed vocabulary
- it carries a noun chain of more than three words (Rule 2.1)
Never rename for style. Apply two checks first:
- Does the name mirror the API it tests? Then keep it.
test_attach_info_built_once_per_relation
tracks Relation._attach_infos(), and that link beats vocabulary purity.
- Every rename changes a CI test id. Flag each one on its own. Afterwards, grep the repo for
the old name.
Workflow
1. Audit
Propose. Do not edit.
Split the diff across subagents by area. File sets must not overlap — two agents writing one
file corrupt each other.
Give each agent its base ref, its file list, this vocabulary, and the classification table. Tell it
to read references/simple-english.md itself, so rule numbers come from the file.
Collect five groups. Give every finding a file:line, rule number, current text, rewrite:
- Vocabulary violations
- Structural violations
- Identifier renames
- Content corrections — false statements. Run the code before you report one.
- Test assertions that break — see the trap below.
Write the proposal to a file. A few hundred findings do not fit in a chat reply. Report the counts
and the open decisions.
Stop. Wait for approval. When the vocabulary does not settle a decision, ask.
2. Apply, in two phases
Docstrings and message strings share files, so they cannot run together.
- Phase A: docstrings, comments, renames, assertion fixes, content corrections.
- Phase B: message strings.
Tell every Phase B agent that the line numbers moved. It must find each string by content. Pin any
substring a test asserts.
Every applying agent obeys four rules:
- Apply only your own findings, only in your own files.
- Never touch code. Not a signature, not a return type, not a call site. Agents break this
rule, and a functionally-equivalent change passes every test. Verify it afterwards.
- Preserve every f-string placeholder: same names, same count, same order.
- Keep lines under 100 chars. Black does not reflow a string or a comment.
3. Verify
Run every step. Each one caught a real defect.
a. Prose-only check. Compare the AST with docstrings stripped, against the branch tip.
Comments never reach the AST, so any difference means code or a string changed. Explain every file
that appears. Against the merge-base the whole PR appears and the signal is lost.
import ast
def strip(t):
for n in ast.walk(t):
if isinstance(n, (ast.Module, ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
n.body = [x for x in n.body if not (isinstance(x, ast.Expr)
and isinstance(x.value, ast.Constant) and isinstance(x.value.value, str))]
return t
b. The four-step self-check from references/simple-english.md, over the added lines. Add a
sweep for the excluded vocabulary.
c. make format. Black reports no change when the agents did their job.
d. make lint.
e. Run the tests. This step is not optional.
The trap: a message rewrite breaks test assertions
Tests match on error text. A grep for the phrases you changed misses some. A parametrized test
hides its assertion far from the message.
One pass found them in two rounds. A grep found five. The test run then surfaced eight more. One of
those was match="cannot be determined". The frame sentence had become "dlt cannot determine the
data location".
- Grep
tests/ for match=, in str(exc, and in str(reject.
- Check the negative assertions too.
assert "can join" not in ... still passes against
"cannot join", because no space follows can.
- Run the suites anyway. The grep is a head start, not the check.
Adding a new term or a new group
Never just record a term. Take these eight steps.
- Pick the group. No group fits? Add one. Give it the next
G<n>, an index entry, and the
three parts: included, excluded, rulings. A group without excluded terms is not finished.
- Research the usage. Grep
dlt/, tests/ and docs/ for the word and every synonym. Count
the hits. Read enough to find the meanings in play. One word often covers two concepts.
- Derive the banned set. A term is useless without one. For each synonym and inflection,
decide: banned, or legal with another meaning? Both halves go in the group.
- Check the upstream interface. The vocabulary must not contradict a public dlt method or
type name.
access won because needs_attach already said "accesses its data". attach info
won because the type is TAttachInfo.
- Name the false positives.
descriptor is banned for TAttachInfo and correct for the
Python descriptor protocol. Put the exception in the rulings, or the next run "fixes" it.
- Fix the part of speech. Say noun or verb, and ban the other use.
access is a verb, so
"data access" is a violation.
- Test the ban before you write it. Grep for the word you intend to exclude. Compliant prose
already uses it? Then the ban is too wide. Narrow it to the meaning you mean.
inaccessible
and foreign-folded passed this check and stay legal. casefold and attach instructions
failed it and are banned.
- Update this file. Add rows to both tables. Add a ruling when the term has a legal exception.
State what becomes newly compliant and what becomes newly non-compliant. A term that flips
direction turns compliant prose into findings.
Amend the vocabulary mid-review when you must. Then tell every running agent what inverted. They
audited under the old table.
Deliverable
- The proposal file, with counts and your open decisions.
- After approval: the edits, the verification output, and what you did not apply.
- Never commit. The maintainer commits.