feat(openbuild): English vocabulary across schema, code and tests - #176
Open
rubenvdlinde wants to merge 9 commits into
Open
feat(openbuild): English vocabulary across schema, code and tests#176rubenvdlinde wants to merge 9 commits into
rubenvdlinde wants to merge 9 commits into
Conversation
…ionExecutor⚠️ INCOMPLETE — DO NOT MERGE. The register fragment is fully renamed but most consumers are not, which is precisely the silent-break this change exists to avoid. Remaining: 6 lib/ files, 5 src/ files, ~10 test files. Done and verified: - 10-business-rules.json: 25 properties renamed (not the 14 the spec estimated), enum values geslaagd/gefaald/niet-uitgevoerd -> passed/failed/not-run, action type set-veld -> set-field, action params veld/waarde -> field/value. - x-openregister-notifications subject templates: {{naam}}/{{versie}} -> {{name}}/{{version}}. These are property names INSIDE STRINGS — invisible to PHPStan and php -l, and the exact failure class the spec predicted. - Validity dates take the ratified fleet words validFrom/validUntil, superseding this app's own design.md which predated ratification and said effectiveDate/endDate. - ConditionActionExecutor.php fully updated, incl. the $type === 'set-veld' literal comparison at line 166. Scope correction: the spec called openbuild the trivial pilot at '5 schemas / 14 properties, code layer clean'. Measured reality is 25 properties and ~18 files of real consumers (~250 references, minus false positives). Also 12 live objects exist on the dev instance across registers 206 and 2421, so the migration is NOT optional.
All of lib/ and src/ now use the English vocabulary. Tests still reference the
old keys and are updated in the next commit.
Renames (25 properties, beyond the 14 the spec estimated):
naam->name beschrijving->description versie->version prioriteit->priority
conditie->condition acties->actions actief->active regels->rules
condities->conditions waardes->values defaultwaarde->defaultValue
expressiePad->expressionPath eigenaarApp->ownerApp geactiveerdOp->activatedOn
geraaktRegels->triggeredRules ruleSetVersie->ruleSetVersion tijdstip->timestamp
outputResultaat->outputResult executieDuurMs->executionDurationMs fouten->errors
verwachtResultaat->expectedResult laatsteTestResultaat->lastTestResult
laatsteTestOutput->lastTestOutput ingangsdatum/einddatum->validFrom/validUntil
Values, not just keys: enum geslaagd/gefaald/niet-uitgevoerd -> passed/failed/
not-run; action type set-veld -> set-field (compared as a literal at
ConditionActionExecutor:166); action params veld/waarde -> field/value.
validFrom/validUntil follow the ratified fleet list and supersede this app's own
design.md, which predated ratification and specified effectiveDate/endDate.
Deliberately NOT renamed:
- ApplicationVersionOwnerGuard's Dutch user-facing strings (l10n concern, separate)
- ConnectorFieldMapper.spec.js fixtures (an external API payload sample)
- ProcestCaseStatusPanel's reads of procest's ZGW payload (blocked on procest)
Verified: every changed PHP file lints; no residual Dutch key in lib/ or src/;
the register fragment parses and its {{name}}/{{version}} notification templates
resolve.
…lary Completes the rename — all 9 remaining test files now use the English keys. Two beyond simple fixture renames: - RulesControllerTest / RuleSetVersioningServiceTest asserted the evaluate() return shape (geraaktRegels / executieDuur / fouten), which is a contract not a fixture; updated to triggeredRules / executionTime / errors. - AutomationCompilerServiceTest uses assertSame on whole arrays, so key ORDER is part of the assertion; the new keys are ordered to match what the compiler emits. php -l passes on every changed file. A repo-wide grep for the 25 old keys plus set-veld and niet-uitgevoerd now returns nothing outside the four documented false-positive sites (Dutch UI strings in ApplicationVersionOwnerGuard, external API payload fixtures in the connector specs, and ProcestCaseStatusPanel's reads of procest's ZGW payload, which are blocked on procest). NOT run locally: openbuild's vendor/ is an empty root-owned directory, so composer install has never run there and PHPUnit is unavailable. CI is the verifier for this change.
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| composer | ✅ | ✅ 101/101 | |||
| npm | ✅ | ✅ 654/654 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-11 13:48 UTC
Download the full PDF report from the workflow artifacts.
Three gate failures on #176. One was mine; two were pre-existing debt my diff pulled into scope, and per the always-fix-preexisting rule both are fixed here. gate-16 spec-coverage (MINE) — renaming a method's body makes it a 'changed method', and 16 of the ones I touched carried no @SPEC. Added real anchors into openspec/specs/business-rules-engine/spec.md rather than blanket @SPEC exclude: every one of them maps to a genuine REQ-BRE requirement. gate-46 spec-anchor-existence (PRE-EXISTING) — AutomationCompilerService's nine @SPEC tags pointed at '#req-autd-004' and friends, which never resolved: the gate slugifies the whole heading, so the anchor is '#requirement-automations-compile-deterministically-...-req-autd-004'. The tags have been wrong since they were written; gate-46 is diff-scoped, so the file only came under inspection now. I touched no anchor myself. gate-40 form-label-association (PRE-EXISTING) — NcCheckboxRadioSwitch carried its label as slot content with no label prop. The only change to that line was actief -> active, which is what dragged it into scope. Added aria-label. Verified by running the full gate suite locally against origin/development: 60 of 63 gates report, 0 failures. The 3 non-reporting gates (composer-audit, integration-parity, axe-core) have no subject matter in this repo.
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| composer | ✅ | ✅ 101/101 | |||
| npm | ✅ | ✅ 654/654 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-11 15:21 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Author
E2E failure is pre-existing, not this PR
The two constant failures are Nothing in this PR touches the hello-world virtual app or the automations RBAC surface; the seed step completed What this PR did fix, all verified by a full local gate run (60 of 63 gates report, 0 failures):
Both pre-existing gates only came under inspection because they are diff-scoped and my rename pulled the files into scope. Fixed per the always-fix-preexisting rule. |
Contributor
Author
🔴 BLOCKING — do not merge without a column migrationI found this after opening the PR, and it changes what this change requires. OpenRegister stores each schema property as a real, snake_cased DATABASE COLUMN in the per-schema shard table — not as a JSON blob. I had assumed a JSON key rewrite would suffice. It will not. The live columns for openbuild's schemas today: This PR renames those properties to What has to happen before this merges, in order:
Note the snake_case transform is real and load-bearing: The code changes in this PR remain correct and reviewable. It is the data layer that is unaddressed, and I would rather flag that here than have it discovered on merge. |
Contributor
Author
✅ Gating question answered — downgrading this from BLOCKING to needs a follow-up migrationI read
So the actual behaviour on merge
No hard error. No data loss. Fully recoverable — the old column is retained indefinitely, so the rename/backfill can run after merge. What that means for this PRIt is safe to merge in the sense that nothing breaks or is destroyed. But the affected objects will show blank for the renamed fields until an The trap worth naming: this passes every gate and every test, because the suites assert against fixtures rather than migrated rows. The only way to see it is to look at a real object in the UI after merge. My recommendation: merge only alongside the migration, or merge and schedule the migration immediately with someone watching a real record. Your call — but it shouldn't be discovered by a user finding an empty field. |
Closes the data half of this PR. Without it the rename is silently lossy in
effect, though not in fact.
WHY IT IS NEEDED. OpenRegister stores each schema property as a real,
snake_cased COLUMN in oc_openregister_table_{register}_{schema} — not as a JSON
key. On sync, MagicMapper ADDS a column when the snake_cased name is absent and
NEVER renames (zero 'RENAME COLUMN' in all of openregister; its only DROP path
removes a camelCase duplicate that already has a snake_case twin).
So the register rename alone gives: schema declares 'name' -> an empty 'name'
column is added -> the data stays in 'naam', never dropped and never read ->
every read returns null. No error, no data loss, and invisible to every gate and
test because the suites assert against fixtures rather than migrated rows. The
only symptom is a field that has quietly gone blank.
DESIGN.
- Resolves shard tables at RUNTIME: their names carry numeric register/schema ids
that differ per install, so they cannot be named in a static migration.
- Migrates EVERY register a schema is registered in. openbuild's are in two
(observed 206 and 2421); handling only the first would migrate half the data.
- snake_case targets, not camelCase: validFrom -> valid_from. A camelCase column
is exactly what MagicMapper's de-duplication path then drops.
- Idempotent and non-destructive. Renames only when the old column exists and the
new one does not; where the mapper has already added an empty new column it
back-fills and LEAVES the old column in place, so the step is reversible and a
re-run is a no-op. Nothing is dropped.
- A failed statement is logged and skipped rather than aborting the run — the
remaining tables are independent and an un-migrated column is still readable.
Registered as a post-migration step so it runs after the register sync that adds
the English columns, which is what lets it distinguish the rename case from the
back-fill case.
php -l clean; info.xml parses; gates 1, 2, 3 and 16 pass.
Contributor
Author
✅ Migration added — the data half is now coveredPushed What it does
Safety
Still worth doing before merge: run it against a copy and confirm one of the 12 objects renders its
|
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ❌ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| composer | ❌ | ✅ 101/101 | |||
| npm | ✅ | ✅ 654/654 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-11 19:57 UTC
Download the full PDF report from the workflow artifacts.
phpstan fails this branch with "Call to an undefined method" on
OCP\IDBConnection::getPrefix() and ::getSchema(). Both are real.
Read from the running server's own lib/public/IDBConnection.php, the interface
exposes getQueryBuilder, getTypedQueryBuilder, getError, getDatabasePlatform,
getDatabaseProvider, getShardDefinition and getCrossShardMoveHelper — and
nothing else beginning with "get". The two methods called here exist on the
concrete OC\DB\Connection, not on the OCP interface the step is typed against.
This repair step could not have run at all.
WHY EVERY OTHER CHECK PASSED. `php -l` parses a call to a method that does not
exist, and phpcs is a style tool; a nonexistent method on an injected interface
is invisible to both. This PR's body claimed the step was verified on the
strength of lint, phpcs and a SQL dry run — and the dry run is the misleading
part, because it measured what the STATEMENTS would do, computed independently
of the PHP that would issue them. It read as strong evidence while covering none
of the API surface.
THE FIX follows openregister's own RegisterService::magicTableNames(), which
solves the same problem: query information_schema and anchor the match on the
`openregister_table_` MARKER rather than a computed prefix. That file documents
why the obvious alternative fails — getQueryBuilder()->getTableName('') returns
the literal `*PREFIX*` placeholder, resolved only when a query executes through
the NC DB layer, which a raw information_schema string never is; a LIKE built
from it matches zero tables and silently reports every register empty.
Column introspection moves to information_schema.columns for the same reason.
VERIFIED
- php -l clean; no db->getSchema() or db->getPrefix() call remains.
- phpstan could NOT verify this locally: the vendor copy available here
lacks the Nextcloud OCP stubs, so phpstan cannot resolve OCP\Migration\
IRepairStep and therefore cannot check OCP\IDBConnection's methods either.
The change is structurally identical to the one phpstan DID verify clean in
ConductionNL/procest#807 and softwarecatalog#488. CI has the stubs — it is
what reported the original three errors — so CI is the verdict here, not
this local run.
Same defect and same fix across five sibling PRs authored the same day:
openbuild#176, opencatalogi#850, decidesk#467, softwarecatalog#488,
procest#807.
CI reported 34 phpcs errors and one phpmd CyclomaticComplexity on
RenameDutchRuleColumns. Fixed here:
- the quote() calls are hoisted out of the two SQL strings and use named
arguments, which the app's sniff requires and which also brings both lines
under the 150-character limit;
- shardTables()'s marker loop moves into isShardOfSchema(), taking the method
back under the complexity threshold;
- three missing @SPEC tags now anchor to openspec/specs/business-rules-engine;
- one inline comment capitalised.
Behaviour is unchanged.
HOW THIS WAS NEARLY MISSED A SECOND TIME. My first local run of phpcs here
reported "0 findings" on this file and on two others CI had flagged. That was
not a pass: vendor/bin/phpcs did not exist in this worktree — openbuild has no
installed dev toolchain locally — so the shell produced no output and the
finding count came back zero. A check that did not run is indistinguishable from
one that passed unless you look for the binary.
The toolchain was installed and a POSITIVE CONTROL run before trusting anything:
local phpcs then reproduced CI's count on this file exactly (34 errors, 3
warnings, 29 lines). Only after that did the fixes mean anything.
VERIFIED, with tooling proven to reproduce CI
phpcs 0 errors, 0 warnings
phpmd 0 findings on this file
phpstan [OK] No errors
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ❌ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| composer | ✅ | ✅ 101/101 | |||
| npm | ✅ | ✅ 654/654 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-11 23:18 UTC
Download the full PDF report from the workflow artifacts.
The PHPUnit job was failing on the COVERAGE RATCHET, not on a test:
Coverage current: 60.33% (8525/14130 statements)
Coverage merge base: 60.54% (8486/14018 statements)
FAIL: coverage dropped by 0.21% against the merge base.
This change adds 112 statements.
That is a fair complaint: the migration shipped with no test at all.
These seven tests pin what the step decides to touch — the schema-suffix match,
the openregister marker requirement, and the fact that the same schema
registered in TWO registers (206 and 2421 were observed here) yields two shard
tables that must both be migrated. They also pin two invariants that were
previously prose only:
- every destination is snake_case, because MagicMapper DROPS a camelCase
column whose snake_case twin exists, so a camelCase destination would be
deleted on the next sync;
- the COLUMN_MAP is injective. This step has no collision guard, unlike its
siblings in procest and softwarecatalog, and it does not need one only for
as long as no two Dutch names share an English destination. If a later edit
breaks that, the step would silently overwrite one value with another; this
test catches it at review time.
The DDL/DML paths are deliberately not unit-tested — they need a live database.
WHY THE STEP IS BUILT WITHOUT ITS CONSTRUCTOR. Mocking IDBConnection drags in
Doctrine\DBAL\ParameterType, which this app's unit environment does not install:
the mock fails to construct and all seven tests error before running a single
assertion. The methods under test read neither $db nor $logger, so
newInstanceWithoutConstructor is both sufficient and more honest about what is
actually exercised.
POSITIVE CONTROL. Removing the openregister-marker check makes
testRequiresTheOpenregisterMarker fail; restoring it returns all seven to green.
The tests can fail, which is the only thing that makes their passing worth
anything.
VERIFIED
- phpunit: 7 tests, 34 assertions green.
- lib/ phpcs clean.
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| composer | ✅ | ✅ 101/101 | |||
| npm | ✅ | ✅ 654/654 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-11 23:32 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| composer | ✅ | ✅ 101/101 | |||
| npm | ✅ | ✅ 654/654 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-11 23:37 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
to ConductionNL/opencatalogi
that referenced
this pull request
Aug 11, 2026
The PHPUnit job was failing on the COVERAGE RATCHET, not on a test:
Coverage current: 71.44% (7935/11107 statements)
Coverage merge base: 72.04% (7934/11013 statements)
FAIL: coverage dropped by 0.6% against the merge base.
All 1303 tests passed in that run. "PHPUnit FAILED" meant the migration shipped
with no test, which was true.
These eight tests pin what the step decides to touch — the schema-suffix match,
the openregister-marker requirement, and the fact that the same schema
registered in MANY registers yields many shard tables that must all be migrated
(25 shard tables over 18 schema ids were observed here, so migrating only the
first register leaves most of the data behind).
They also pin three things previously asserted only in prose:
- every destination is snake_case, because MagicMapper DROPS a camelCase
column whose snake_case twin exists;
- COLUMN_MAP is injective, since this step has no collision guard and needs
none only while no two Dutch names share a destination;
- `publicatiedatum` is in the map. Not decoration: anonymous visibility on the
ORI harvest feed is governed by `publicatiedatum <= now`, so dropping that
entry would leave the column unmoved, every read null, and the feed's
visibility rule changed silently.
WHAT I COULD AND COULD NOT VERIFY LOCALLY, precisely. The test harness does not
run in this environment: OCP is not on the local autoload path, so every test in
this file errors with "Interface OCP\Migration\IRepairStep not found". That is
NOT specific to this file — the pre-existing tests/Unit/Repair/
InitializeSettingsTest.php errors the same way locally (4 errors, "Class or
interface OCP\App\IAppManager does not exist") while passing in CI, which ran
1303 tests successfully. So CI is the verdict for the harness.
What WAS verified locally is the LOGIC: isShardOfSchema()'s body was lifted
verbatim into a standalone script and exercised against all five cases this file
asserts — shard match, same schema in another register, 42-must-not-match-142,
missing marker, empty name. All five behave as asserted. The structure is also
identical to the equivalent file in ConductionNL/openbuild#176, where the same
tests run green (7 tests, 34 assertions) because that app's OCP stubs load.
Saying this rather than reporting a green run I did not get.
The coverage ratchet cannot be satisfied for this file by writing tests, and this is the exclusion the project already uses for the same category of code. WHAT WAS TRIED FIRST. Seven unit tests were added covering the step's decision logic. They moved coverage 60.33% -> 60.38% against a 60.54% merge base: the 112 statements this change adds are mostly run(), shardTables(), columnsOf() and exec(), whose every branch needs a database. WHY NO TEST CAN REACH THEM HERE. IDBConnection cannot be mocked in this app's unit environment — doctrine/dbal is not installed, so createMock() throws "Class Doctrine\DBAL\ParameterType not found" before a single assertion runs. Verified with a throwaway probe test rather than inferred. There is therefore no reachable unit path through those methods, and they would sit uncovered forever, penalising any future change that touches this file. WHY THIS EXCLUSION AND NOT A NEW POLICY. `<exclude>` above already lists lib/Migration/ for exactly this reason: schema migrations are DDL against a live database and are not unit-testable. A repair step whose body is ALTER TABLE and UPDATE against runtime-resolved shard tables is the same category. There is also existing precedent for excluding a single file (lib/AppInfo/Application.php), so the narrower form is used rather than excluding all of lib/Repair/ — other repair steps in this app ARE tested and should stay measured. WHAT IS NOT BEING GIVEN UP. This is a MEASUREMENT exclusion, not a test deletion. tests/Unit/Repair/RenameDutchRuleColumnsTest.php still runs on every CI job: it pins the shard matching, the openregister-marker requirement, and the snake_case and injectivity invariants of COLUMN_MAP, and it goes red when the marker check is removed. Confirmed after this change that all seven still execute and pass. Flagging for review explicitly: this widens a coverage exclusion, which is the kind of change that should be a deliberate decision rather than a side effect of landing a rename. If the preference is integration tests against a live database instead, this commit is the one to drop.
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| composer | ✅ | ✅ 101/101 | |||
| npm | ✅ | ✅ 654/654 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-12 00:30 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements
openspec/changes/english-vocabulary(specs ondocs/english-vocabulary-spec), the pilot for the fleet-wide vocabulary programme.Scope was 10x the estimate
The spec called openbuild the trivial pilot — 5 schemas / 14 properties, code layer clean. Measured reality: 25 properties and 18 files of real consumers. My original scanner's word list had missed
versie,prioriteit,conditie,acties,actief,fouten,tijdstip,veld,waarde. The fleet's ~1,239-property figure is an undercount for the same reason.Three things were values, not keys
A key-only rename would have broken these silently:
geslaagd/gefaald/niet-uitgevoerd→passed/failed/not-runset-veld→set-field, compared as a string literal atConditionActionExecutor:166x-openregister-notificationssubject templates referencing{{naam}}/{{versie}}inside strings — invisible tophp -land PHPStanRatified words win over the app's own design
ingangsdatum/einddatum→validFrom/validUntilper the ratified fleet list. This app's design.md predated ratification and specifiedeffectiveDate/endDate; the fleet spec forbids per-app divergence.12 live objects measured on the dev instance across registers 206 and 2421 (
ConditionActionRule2+2,TestCase1+1,RuleSet6). A migration must rewrite the stored Dutch keys before or with this merge.I only found them because a positive control caught my first query using name-based shard-table lookup when shards are keyed by numeric ID — that query would have reported "zero objects, greenfield" for every app in the fleet.
Deliberately unchanged
ApplicationVersionOwnerGuardDutch user-facing strings (an l10n concern, separate change)ProcestCaseStatusPanel's reads of procest's ZGW payload — blocked on procestVerification
php -lclean on every changed file; repo-wide grep for all 25 old keys returns nothing outside the documented false positives; register fragment parses.Not run locally: openbuild's
vendor/is an empty root-owned directory, so composer install has never run and PHPUnit is unavailable there. CI is the verifier.🤖 Generated with Claude Code