werkator build succeeded after 00:26
See docs/prs/2026-10-05-PR#17-db-schema-judged-by-jev.md. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed-on: #17 Co-authored-by: claude-code <claude-code@javagil.de>
224 lines
9.0 KiB
Markdown
224 lines
9.0 KiB
Markdown
# PR#17: Jev judges the risk of every changed Liquibase changeset
|
||
|
||
> **WARNING:** This document describes only the change applied in this PR.
|
||
> It may already be outdated once the next PR is merged.
|
||
> Historic PR documentation is not maintained along with new PRs; treat it as a snapshot.
|
||
|
||
## The Problem
|
||
|
||
werkrevisor 0.15.0 gives YELLOW to every PR that changes a DB schema path.
|
||
In hsadmin-ng this concerns 12 of 25 measured PRs, also for a new table or a new nullable column.
|
||
|
||
hsadmin-ng deploys through several stages, never directly to production,
|
||
and a restore of the database always reverts a deployment.
|
||
So only a changeset that destroys data needs a human before the merge.
|
||
Other repositories may deploy directly, so the operator decides per host.
|
||
|
||
The built-in DB schema paths also match test classes in a `migration` package,
|
||
e.g. `LiquibaseCompatibilityIntegrationTest.java`, although they change no schema.
|
||
|
||
A measurement on 34 changesets of 11 hsadmin-ng PRs, rated by hand on four levels, gave:
|
||
|
||
| Hand rating | Changesets | Jev risk, mean (0 to 3) | Range |
|
||
|---|---|---|---|
|
||
| 0 purely additive | 11 | 0.55 | 0.00 to 1.40 |
|
||
| 1 DB code or reference data | 14 | 1.38 | 0.35 to 2.00 |
|
||
| 2 access control or existing data | 8 | 1.86 | 1.43 to 2.04 |
|
||
| 3 data loss | 1 | 2.99 | – |
|
||
|
||
Only the dropped column of hsadmin-ng PR#310 reached a risk of 2.5 or a data-loss probability of 0.6 (2.99 and 0.84).
|
||
|
||
## Non-Goals
|
||
|
||
- No Jev judgment by default: without `[db_schema] judge = "jev"` every DB schema change stays YELLOW.
|
||
- No repository setting: only the host knows how a repository deploys.
|
||
- No ORANGE for a risky changeset: a risky change is no finding.
|
||
- No parser for XML, YAML or JSON changelogs: such a file is judged as a whole diff.
|
||
|
||
## The Scenarios
|
||
|
||
### Feature: the host decides how DB schema changes count
|
||
|
||
#### Scenario#17.01: without a judge every DB schema change gives YELLOW
|
||
|
||
So that a repository without staged deployment keeps the safe default.
|
||
|
||
- **Given** the host config has no `[db_schema]` section, or `judge = "never"`
|
||
- **When** a PR adds a new table in a Liquibase changeset
|
||
- **Then** the verdict is YELLOW with the reason `db_schema`
|
||
|
||
##### Verified by
|
||
|
||
- [test_dbschema.DbSchemaFlowTest.test_never_judge_gives_yellow](../../tests/test_dbschema.py)
|
||
|
||
#### Scenario#17.02: Jev's two answers decide the color of a changeset
|
||
|
||
So that only a changeset that can damage production data needs a human.
|
||
|
||
- **Given** the host config sets `[db_schema] judge = "jev"` with the defaults `risk_min = 2.5` and `data_loss_min = 0.6`
|
||
- **and** a PR adds one changeset
|
||
- **When** Jev rates the deploy risk with `<risk>` of 3 and the data-loss probability with `<loss>`
|
||
- **Then** the result is `<result>`
|
||
|
||
| risk | loss | result |
|
||
|---|---|---|
|
||
| 0.2 | 0.05 | GREEN with the hint `db_schema_judged` |
|
||
| 2.0 | 0.38 | GREEN with the hint `db_schema_judged` |
|
||
| 2.99 | 0.84 | YELLOW with the reason `db_schema_risky` |
|
||
| 1.0 | 0.70 | YELLOW with the reason `db_schema_risky` |
|
||
|
||
##### Verified by
|
||
|
||
- [test_dbschema.DbSchemaFlowTest.test_jev_answers_decide_the_verdict](../../tests/test_dbschema.py)
|
||
|
||
#### Scenario#17.03: Jev judges each changeset on its own, a changed one with its old version
|
||
|
||
So that Jev sees what a changeset does to an existing database.
|
||
|
||
- **Given** a PR adds a new changeset and changes an existing repeatable changeset
|
||
- **When** werkrevisor asks Jev
|
||
- **Then** Jev judges each of the two changesets on its own
|
||
- **and** Jev also sees the old version of the changed changeset
|
||
|
||
##### Verified by
|
||
|
||
- [test_dbschema.ChangesetTest.test_units_hold_new_and_changed_changesets](../../tests/test_dbschema.py)
|
||
|
||
#### Scenario#17.04: a changed or removed released changeset gives YELLOW without Jev
|
||
|
||
So that a checksum conflict on an existing database never passes.
|
||
|
||
- **Given** the host config sets `[db_schema] judge = "jev"`
|
||
- **When** a PR `<change>`
|
||
- **Then** the verdict is YELLOW with the reason `db_schema_released_changed`
|
||
|
||
| change |
|
||
|---|
|
||
| changes the changeset `a:table` without `runOnChange:true`, `runAlways:true` or `--validCheckSum: ANY` |
|
||
| removes the changeset `a:table` |
|
||
| deletes a changelog file |
|
||
| adds a second changeset with the id `a:table` to the same file |
|
||
|
||
##### Verified by
|
||
|
||
- [test_dbschema.ChangesetTest.test_released_changes_are_found](../../tests/test_dbschema.py)
|
||
- [test_dbschema.DbSchemaFlowTest.test_released_changeset_change_gives_yellow](../../tests/test_dbschema.py)
|
||
- [test_dbschema.DbSchemaFlowTest.test_base_is_the_merge_base](../../tests/test_dbschema.py)
|
||
|
||
#### Scenario#17.05: a file without changesets is judged as a whole diff
|
||
|
||
So that a master changelog or a Flyway migration does not escape the judgment.
|
||
|
||
- **Given** the host config sets `[db_schema] judge = "jev"`
|
||
- **When** a PR adds an include line to `db.changelog-master.yaml`
|
||
- **Then** Jev gets the diff of the file as one state
|
||
|
||
##### Verified by
|
||
|
||
- [test_dbschema.ChangesetTest.test_file_without_changesets_is_one_unit](../../tests/test_dbschema.py)
|
||
|
||
#### Scenario#17.06: a changeset that Jev cannot judge gives YELLOW
|
||
|
||
So that a failed or skipped call never hides a risky change.
|
||
|
||
- **Given** the host config sets `[db_schema] judge = "jev"`
|
||
- **When** `<cause>`
|
||
- **Then** the verdict is YELLOW with the reason `db_schema_not_judged`
|
||
|
||
| cause |
|
||
|---|
|
||
| Jev fails |
|
||
| the changeset is larger than `[classifier] max_state_chars` |
|
||
| the version at the merge base cannot be read or is not UTF-8 text |
|
||
| the diff holds a possible secret |
|
||
| werkrevisor runs with `--no-llm` |
|
||
|
||
##### Verified by
|
||
|
||
- [test_dbschema.DbSchemaFlowTest.test_unjudged_changesets_give_yellow](../../tests/test_dbschema.py)
|
||
- [test_dbschema.ChangesetTest.test_unreadable_base_or_duplicate_ids_are_never_judged_as_new](../../tests/test_dbschema.py)
|
||
|
||
#### Scenario#17.07: test files are no DB schema
|
||
|
||
So that a test of the database migration does not count as a change of the database.
|
||
A repository cannot declare its own changelogs as tests.
|
||
|
||
- **Given** a test lies in a test directory, in a folder named `migration`
|
||
- **When** a PR changes this test
|
||
- **Then** werkrevisor does not report a DB schema change
|
||
|
||
##### Verified by
|
||
|
||
- [test_change_kinds.DbSchemaTest.test_test_files_are_no_db_schema](../../tests/test_change_kinds.py)
|
||
|
||
#### Scenario#17.08: the usage record holds Jev's values for every changeset
|
||
|
||
So that the thresholds can be calibrated later.
|
||
|
||
- **Given** Jev judged two changesets
|
||
- **When** werkrevisor writes the usage record
|
||
- **Then** the record lists both with file, changeset id, whether new, risk, data-loss probability and color
|
||
|
||
##### Verified by
|
||
|
||
- [test_dbschema.DbSchemaFlowTest.test_usage_record_holds_the_jev_values](../../tests/test_dbschema.py)
|
||
|
||
#### Scenario#17.09: only a valid host config can switch on the DB schema judgment
|
||
|
||
So that a PR cannot weaken the DB schema rule of its own repository.
|
||
|
||
- **Given** the `<layer>` sets `<setting>` in the section `[db_schema]`
|
||
- **When** werkrevisor starts
|
||
- **Then** werkrevisor stops with a config error
|
||
|
||
| layer | setting |
|
||
|---|---|
|
||
| repository config | `judge = "jev"` |
|
||
| host config | `judge = "maybe"` |
|
||
| host config without a `[classifier]` section | `judge = "jev"` |
|
||
| host config | `risk_min = 4` |
|
||
| host config | `data_loss_min = 1.5` |
|
||
|
||
##### Verified by
|
||
|
||
- [test_dbschema.DbSchemaConfigTest.test_db_schema_is_host_only_and_validated](../../tests/test_dbschema.py)
|
||
|
||
## Measurement
|
||
|
||
Dry run of `config/hsadmin-ng.toml` with Jev, but without the model, on 25 merged hsadmin-ng PRs, #265 to #315:
|
||
|
||
| Verdict without the model | 0.15.0 rules only | 0.16.0 with Jev |
|
||
|---|---|---|
|
||
| GREEN | 7 | 9 |
|
||
| YELLOW | 18 | 16 |
|
||
|
||
#301 and #303 become GREEN: their only YELLOW reason was the DB schema.
|
||
#310 stays YELLOW with `db_schema_risky` for its dropped column (risk 2.99, data loss 0.86).
|
||
The other DB PRs stay YELLOW for RBAC paths, size or the PR document.
|
||
|
||
A first dry run compared with the current head of the base branch;
|
||
changesets added there later counted as removed, e.g. in #281, #307 and #310.
|
||
Since then werkrevisor compares with the merge base.
|
||
|
||
## The Solution
|
||
|
||
- The optional host section `[db_schema]` has `judge` (`"never"` or `"jev"`), `risk_min` (2.5) and `data_loss_min` (0.6).
|
||
- The new module `werkrevisor/dbschema.py` splits Liquibase formatted SQL at `--changeset` lines
|
||
and compares the changesets of the head and the merge base by their id;
|
||
so a changeset added to the base branch after the branch point does not count as removed.
|
||
- A changed non-repeatable or a removed changeset, and a deleted schema file, give YELLOW without Jev.
|
||
- Jev answers the two questions of the measurement per new or changed changeset;
|
||
the comment lists the judged changesets as a hint, the usage record logs the values.
|
||
- Files below `src/test/`, `test/` or `tests/` never count as DB schema;
|
||
the paths are built in, so a repository cannot declare its changelogs as tests.
|
||
- `config/hsadmin-ng.toml` sets `judge = "jev"`; the other example configs keep `"never"`.
|
||
- werkrevisor 0.16.0.
|
||
|
||
## Prerequisite PRs
|
||
|
||
- PR#16: the colors say how certain a finding is.
|
||
|
||
## Follow-up PRs
|
||
|
||
- Calibrate `risk_min` and `data_loss_min` against the logged Jev values.
|