Files

Chris Truter ade2ab95fa Give :discouraged-var and :discouraged-namespace their own per-symbol ratchet budgets (#83352 )

* Remove stale :discouraged-namespace ignores

These six ignores suppressed a namespace-level policy that no longer
exists in config.edn -- clojure.pprint, clojure.java.io, clj-yaml.core,
metabase.util.jvm, and metabase.config.core aren't configured as
discouraged namespaces anymore (clojure.pprint is only discouraged at
the var level now). Verified clean with a scoped kondo run requesting
:discouraged-namespace at :warning over just these files: zero findings.

* Give :discouraged-var and :discouraged-namespace their own per-symbol ratchet budgets

One shared budget per linter let a new ignore on a previously-clean var
land silently as long as the linter's aggregate count had room, and made
"introduce a policy for just this one var" impossible. Each configured
symbol now gets its own field: :discouraged-var-counts and
:discouraged-namespace-counts, siblings of :ignore-counts, keyed to
mirror the symbol's own namespace and name rather than the linter
that discourages it (metabase.entity-retrieval.core/search-unfiltered
-> :entity-retrieval.core/search-unfiltered; an enterprise namespace of
the same shape gets an `ee.` prefix so it can't collide with its OSS
counterpart).

Attribution resolves which symbol an inline ignore covers by running a
scoped `clojure -M:kondo` analysis pass over the files that carry one of
these two ignores (JSON output, since a generated var can have a name
that isn't valid EDN) and matching each ignore to the nearest covered
usage of a known symbol -- this is the one thing in the ratchet tooling
that needs a live kondo run, so `./bin/mage kondo-ratchets` is no longer
pure Babashka. `:comment-exempt` is untouched: occurrences still carry
the bare linter name in :linters, only the two new count fields are
keyed per symbol.

`--seed :discouraged-var`/`:discouraged-namespace` (no specific symbol)
seeds every symbol currently configured under it in one shot, unused
ones at zero, the same ergonomics as landing a new linter. A stale flat
:ignore-counts entry for either linter is self-healing: `fix!` drops it
every run and `check` fails if one reappears, since it would otherwise
double-count every occurrence the new field already covers.

Ported onto the prod/test ratchet split: both ratchet files carry the
per-symbol fields, and each file's occurrences are attributed separately.

* Attribute discouraged-var ignores from kondo findings, not nearest usage

Matching each ignore to the nearest later usage of a configured symbol
counted usages kondo never flagged (scoped :config-in-ns entries, :off
overrides) and let a stale ignore claim an unrelated usage. Attribution
now lints copies of the files with the discouraged ignores disabled,
joins each finding to its var or namespace usage at the same row and
column, and charges it to the last ignore before it naming the linter.
An ignore counts once per file and distinct symbol it covers. The copies
rename the ignore key to one of the same length rather than blanking the
ignore, so every form reads as before and every row and column stays put.

A failed kondo run now throws instead of zeroing every budget. Findings
that resolve to no configured symbol fail the check and stop the shrink
from writing; ignores that cover no finding only warn. Without an
:attribute, a discouraged ignore throws rather than reading as zero.
Each ratchet file's occurrences are attributed separately.

`--seed :discouraged-var/clojure.core/println` seeds one symbol, a bare
`--seed :discouraged-var` seeds every symbol at its actual count, and an
empty suffix is rejected. Bulk seeding at zero is gone: the next shrink
dropped those zeros anyway. The shrink workflow runs prepare-backend,
since the shrink now needs the Clojure CLI.

Budgets are re-seeded from the new attribution. Several were undercounted
or charged to the wrong symbol before.

This squashes the unpushed review-fix commits on top of the per-symbol
commit, since #83383 rewrote the same functions each of them changed.

* Remove a stale :discouraged-namespace ignore on clj-yaml.core

clj-yaml.core is no longer a discouraged namespace, so the ignore on its
require in lib-be.metadata.jvm suppresses nothing.

* Don't charge findings kondo reports with the ignores in place

Attribution charged every discouraged finding on the disabled-ignore
copies to the last ignore before it, including findings no ignore
suppresses. Unsuppressed usages in dev/ inflated the eval, println and prn
budgets, and a new usage of a symbol an earlier ignore already covers
went uncounted.

Lint the original files in the same kondo run and drop any finding they
also report, so only findings an ignore suppresses are attributed. An
ignore nested inside another for the same linter still takes the
findings under it, leaving the outer one reported as covering nothing.

* Parse kondo's JSON output with babashka.json

mage already depends on babashka.json, so attribution doesn't need
cheshire or the :discouraged-namespace ignore that came with it. That
returns the cheshire.core budget to 4.

* Attribute prod and test ignores in one kondo run

check and fix! attributed the prod and test occurrences in separate
calls, and each call started its own JVM and lint pass. :attribute now
takes a seq of occurrence groups and returns one attribution per group,
so attribute-occurrences! lints every file once and splits the result.
kondo-ratchets drops from about 39s to 33s.

* Drop budgets for symbols no longer configured instead of refusing

Removing a symbol from config.edn left its per-symbol budget naming an
unknown key, and both check and the shrink threw on it. The shrink, the
tool meant to repair that drift, couldn't. Two PRs could each pass alone
and break the shrink on master: one seeding a symbol, the other removing
it from the config. check now warns about such a budget, and the shrink
drops it at zero like any unused budget, since attribution never charges
an unconfigured symbol. --seed still rejects an unconfigured symbol.

A bare --seed :discouraged-var no longer warns once per configured symbol
with no ignores, and check's ok line counts the per-symbol budgets
alongside :ignore-counts.

* Share the ratchet setup and config walks between their callers

check and fix! repeated the same setup: validate the linters, read the
module and test ratchet files, scan, split prod from test, and attribute.
They now share read-policies and scan-and-attribute. fix! still resolves
and validates --seed before the scan, so a bad seed fails without waiting
on kondo.

The repository-linter and discouraged-symbol lookups each walked
config.edn with an atom to collect nested :linters maps. Both now read
them from one linters-maps. The budget-line formatter is shared, and the
empty and mergeable field sets derive from ratchet-field-order. Dropped
the unused one-argument discouraged-symbols.

* Correct the ratchet task's runtime claim and plain up its wording

The kondo-ratchets :doc said the check takes seconds; with the kondo
attribution pass it takes about half a minute. The CLAUDE.md paragraph
and the ratchet-file headers no longer open sentences with code.

The unattributed-ignore warning now names a nested ignore for the same
linter as a cause: the inner ignore takes the finding, so the outer one
reads as covering nothing even though removing it would surface a
warning.

* Test that a failed kondo run throws rather than reading as no findings

If a failed or garbled kondo run read as an empty result, attribution
would count zero ignores and the shrink would drop every per-symbol
budget. The exit and parse checks move into parse-kondo-output, which
tests now cover for a nonzero exit and for output that isn't a map.

The attribution tests build matching findings and var usages with one
var-hits helper, and the check test's attribution stub reads as what it
returns.

* Refuse colliding discouraged symbols before an unseeded shrink

Dropping the unknown-key validation also dropped the shrink's only call
to discouraged-count-keys, which throws when two configured symbols share
a ratchet key. An unseeded shrink could then merge both symbols' ignores
into one budget. read-policies now reads the configured keys up front, so
check and fix! both refuse a collision before scanning.

* Reject kondo output without a findings list

A run that exits cleanly but prints a map with no :findings, such as {},
parsed as a result with no findings. Attribution would then count zero
ignores and the shrink would drop every per-symbol budget.
parse-kondo-output now requires :findings to be a list. A missing
analysis bucket already fails loudly, since every finding comes back
unresolved.

* Note where nested ignores leave a budget short

Attribution charges a finding to the last ignore before it, so an outer
ignore around a nested one for the same linter counts toward no budget.
State that next to the rule, where a reader of the code will look for it.

* Flag the nested-ignore undercount as a known gap

Lead the comment with the gotcha and its consequence, a budget one short
that an unattributed warning hides behind, so a reader doesn't mistake
the last-ignore rule for a complete one.

* Key aliased discouraged vars by the namespace the file requires

Kondo resolves a re-exported var to the namespace it learned the var came
from, and what it has learned depends on its cache and on which files
share the run. lib.core re-exports ->legacy-MBQL from lib.convert, so
after an ordinary lint of lib/core.cljc the same ten ignores moved from
:lib.core/->legacy-MBQL to :lib.convert/->legacy-MBQL and the check
failed.

A var used through an alias now takes its namespace from the file's own
require for that alias, per language so a .cljc file's clj and cljs
requires stay apart, and the attribution lint runs with --cache false.

* Split the attribution test by behaviour

One deftest each for counting, which findings are charged, and how a
finding resolves to a symbol, so a failure names the behaviour that broke.

* Say a stale flat discouraged budget is never read, not double-counted

The check removes these linters from the occurrences before comparing
:ignore-counts, so a leftover flat entry is ignored rather than counted a
second time.

* Mark the nested-ignore gap as a way past the ratchet, with a TODO

A new ignore wrapped around an existing one for the same linter needs no
budget, so say that plainly and record the planned fix: lint copies with
one ignore disabled each and charge a finding to the copy that reveals it.

2026-10-02 22:42:32 +00:00

7.4 KiB

Raw Permalink Blame History

Metabase Development Guide

Skills

For detailed guidance on writing and reviewing code and documentation, see the skills in .claude/skills/:

Clojure

clojure-mcp tools

clojure-eval - Always use this to evaluate Clojure code, run tests, and verify edits/compile. Prefer this over shell commands.
clojure-write - Clojure/ClojureScript development with REPL-driven workflow and coding conventions
clojure-review - Clojure/ClojureScript code review guidelines and style enforcement

clojure-mcp-lite tools

clj-nrepl-eval - This is another good mechanism for running Clojure code on an nrepl server.

TypeScript

typescript-write - TypeScript/JavaScript development and best practices
typescript-review - TypeScript/JavaScript code review guidelines

Documentation

docs-write - Documentation writing with Metabase style guide
docs-review - Documentation review checklist

Serialization

serdes-workflow - Export, validate, and import Metabase content via serdes
serdes-yaml-edit - Edit exported YAML files with correct portable references

Frontend

analytics-events - Add product analytics events to track user interactions

Important: When working with frontend code, read frontend/CLAUDE.md for project-specific guidelines on component preferences, styling, testing requirements, and available scripts.

Running Backend Tests

If you do not have clojure-eval available to you or clj-nrepl-eval, do not fall back to clj -X:dev:test directly. Instead, use ./bin/test-agent. It produces clean, plain-text output with no progress bars or ANSI codes.

./bin/test-agent :only '[metabase.foo-test]'              # run a namespace
./bin/test-agent :only '[metabase.foo-test/some-test]'    # run a single test
./bin/test-agent :only '[metabase.foo-test metabase.bar-test]'  # multiple namespaces

For module-scoped runs — useful when validating a branch's blast radius — pass :module (single) or :modules (vector) to scope tests to the module(s) the branch touched. The test runner resolves each module to its test directory through its :ns-prefix: lib.schema → test/metabase/lib/schema, enterprise/foo → enterprise/backend/test/metabase_enterprise/foo (see metabase.test-runner/module-folders).

./bin/test-agent :module enterprise/workspaces
./bin/test-agent :modules '[sql-parsing query-processor]'
# Driver tests: --drivers=LIST adds the driver aliases and sets DRIVERS=LIST in one step.
./bin/test-agent --drivers=mysql,h2,postgres :module enterprise/workspaces

Once again, do not use clj -X:dev:test directly — its progress-bar output is hard to parse.

Module Boundaries

The linter config at .clj-kondo/config/modules/config.edn records each module's :api, :uses, :model-exports, and :model-imports. metabase.core.modules-test fails when it drifts from the source.

After any backend change that could shift module boundaries, regenerate it:

./bin/mage fix-modules-config

Changes that shift boundaries include: adding/removing/renaming a src namespace, adding or dropping a cross-module require or :model/X reference, or creating a new module. When unsure, just run it — it is a no-op (exits unchanged) when nothing drifted.

It piggybacks on a running dev nREPL (~5s) and auto-spawns a JVM if none is running (~15s). It only edits the four generated keys; structural changes it can't safely make (a new module needs a human :team, or modules need reordering) are printed as WARNING: lines for you to resolve by hand.

Run all repository-level checks, or one named suite:

./bin/mage project-tests
./bin/mage project-tests <backend|migrations|modules|ratchets>

Nested modules

Module names form a tree: lib.schema is a child of lib. When OSS module search exists, enterprise/search is its child. Run ./bin/mage modules-tree to inspect the hierarchy, or add --html for an interactive explorer with a dependency graph.

Namespace ownership uses the most specific matching prefix. Declaring lib.schema assigns metabase.lib.schema.* to it without moving files. Use :ns-prefix when namespaces do not match the module name.
Every cross-module dependency still requires :uses.
A child may use an ancestor's internal namespaces. Parents, siblings, and unrelated modules must use the target's :api.
Each :module-exports entry widens a nested module's visibility by one ancestor. Export every link to make it available everywhere. OSS module X exports its enterprise/X companion automatically.

Ratchets

After changing Clojure suppressions or module-boundary escape hatches, run ./bin/mage kondo-ratchets. Fix the underlying issue when possible; suppressions are a last resort and need a nearby explanation.

Inline-ignore budgets (:ignore-counts) are split by prod/test so one side's suppressions can never mask or look equivalent to the other's: .clj-kondo/ratchets.edn covers production code, and .clj-kondo/ratchets-test.edn covers anything under a test directory. :config-counts (.clj-kondo/config.edn suppressions) stays combined in .clj-kondo/ratchets.edn regardless -- see that file's own header for why. .clj-kondo/config/modules/ratchets.edn (module-boundary escape hatches) is a separate, unsplit concern.

Explain budget increases in the PR. Do not record reductions in feature PRs: post-merge automation opens a Tighten ratchets PR for them. Use ./bin/mage kondo-ratchets-shrink --seed :linter only when adding an inline-ignore budget. If any ratchet file conflicts, run ./bin/merge-kondo-ratchets.

.clj-kondo/config/modules/cycle-clusters.edn names each cyclic cluster (strongly connected component) of the module require graph by an anchor module: the cluster holding the anchor carries the name, and its members are computed, so a named cluster can grow and shrink without an edit. The big one is galactic-center. Names follow a space theme; the name-module-cycle skill helps pick one.

The test metabase.core.module-cycles-test fails until every cluster holds exactly one anchor. When you split a cluster, name the new half: the failure proposes a name and anchor to add. When two named clusters merge, find another way; the failure shows the requires joining them. When an anchor leaves every cycle, anchor its name on another member if the cluster survives, or remove the line.

Ignores for :discouraged-var and :discouraged-namespace are budgeted per symbol, not per linter, in both files. Use --seed :discouraged-var/clojure.core/println to seed one symbol's budget, or a bare --seed :discouraged-var (or :discouraged-namespace) to seed every symbol configured under that linter. To learn which symbol an ignore covers, ./bin/mage kondo-ratchets runs clojure -M:kondo over the files carrying one, so it needs the Clojure CLI as well as Babashka.

Tool Preferences

If clojure-mcp tools are available, prefer them over shell-based alternatives for Clojure development.

7.4 KiB Raw Permalink Blame History