Review Vocabulary

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

1. 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.
2. references/simple-english-use-cases.md — needed for the error-message shape.
3. 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:

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.

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:

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:

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

Excluded

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

Excluded

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

Excluded

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

Excluded

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

Excluded

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:

1. Vocabulary violations
2. Structural violations
3. Identifier renames
4. Content corrections — false statements. Run the code before you report one.
5. 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
# compare ast.dump(strip(ast.parse(old))) with ast.dump(strip(ast.parse(new)))

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.

1. 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.
2. 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.
3. 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.
4. 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.
5. 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.
6. Fix the part of speech. Say noun or verb, and ban the other use. access is a verb, so "data access" is a violation.
7. 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.
8. 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.