Skip to content

ci: make clean Symfony bootstrap structurally checkable - #10

Open
phibkro wants to merge 12 commits into
mainfrom
publish/0002-clean-bootstrap
Open

ci: make clean Symfony bootstrap structurally checkable#10
phibkro wants to merge 12 commits into
mainfrom
publish/0002-clean-bootstrap

Conversation

@phibkro

@phibkro phibkro commented Aug 12, 2026

Copy link
Copy Markdown
Member

Design spec

design-specs/0002-symfony-clean-checkout-bootstrap.md

Journey

A maintainer checks out the repository without cached PHP artifacts and runs the bounded Symfony bootstrap workflow. The workflow classifies the current PHPUnit, lint, and analysis results structurally instead of pinning transient result tuples.

Experience it

  1. Check out this branch cleanly.
  2. Run the commands and clean-checkout scenario recorded in the spec Evidence section.
  3. Inspect the CI workflow output and the current red-baseline classifications.

Evidence

  • Clean-checkout bootstrap/runtime evidence: PASS for the bounded bootstrap.
  • Workflow no longer treats the historical 1167/55/3/3156 tuple as an acceptance constant.
  • Exact PR scope: final 0002 spec plus .github/workflows/ci.yml.

What is real

The bootstrap and structural result classification are real and locally verified. PHPUnit still has 2 failures, lint has 8 findings, and analysis has 1 finding; this PR does not claim a green Symfony suite, migration replay, deployment, or production acceptance.

phibkro and others added 12 commits August 10, 2026 02:03
`turbo build` was red at HEAD: config.ts reads import.meta.env for the
Vite-bundled path, which plain tsc has no type for.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Vendored fork of a superseded Express/Drizzle/Postgres repo. Nothing
consumed it — the SDK talks only to the Symfony server.

Removed with it: the docker-compose api + postgres services, the CI
postgres service container and drizzle-migrate step, the DATABASE_*
passThroughEnv (api-only), the dev:api script, the pnpm-lock sync in
pre-commit, and the README/CLAUDE.md entries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Left over from the openapi-fetch client. The hand-written domain client
does not reference it: tsconfig includes only src/, package.json ships
only dist/, and no import or type reference points at it. Drops the
oxlint ignore that existed solely for it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The README still described the deleted openapi-fetch client
(createClient(...).GET(path), createQueryApi, `turbo run generate`).
Replace it with the actual hand-written domain-first Effect client:
domain methods, dual promise/Effect export, SdkError hierarchy, and
the Symfony adapter's date/status normalisation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Receipt::$visualId is declared unique in the database but was generated as
dechex(milliseconds), so every receipt built inside the same millisecond got
the same id. Eight receipts constructed back to back produced one distinct id,
which is why loading the fixtures aborted on receipt.visual_id.

Appending four random bytes keeps the time-ordered hex prefix humans read in
emails and receipt tables while making the value unique. No consumer parses the
format -- the templates echo it and the SDK types it as a plain string.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… data

Guard-parity items A1, A2 and A7 declared uniqueness invariants that a scan of
the production backup refutes. In each case the stated key would make valid
data unrepresentable, so the entity is corrected rather than the data:

  school_capacity   + department_id  one school has capacity in one semester
                                     under two departments (3 collisions -> 0)
  team_membership   + position_id    one person holds two positions in one team
                                     and semester (6 of 13 collisions are valid)
  assistant_history + bolk           a semester has two teaching blocks and an
                                     assistant is placed once per bolk

The three constraint tests asserted the old keys, so they asserted rules the
domain does not have. Each now tests both directions: the duplicate that must
be rejected, and the case the old key would have wrongly rejected.

A2 and A7 still cannot be enforced in the database. A7 has 7 true duplicates
left; A2 needs a data-shape migration first, because 332 rows store "both
bolks" as a single 'Bolk 1, Bolk 2' row rather than two rows. The cleanup
queries, the split logic and the follow-up migration are written down in
docs/migrations-blocked-unique-constraints.md so the gap is visible instead of
silent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aints

Guard-parity items A1, A2, A6 and A7 added four uniqueness invariants as ORM
metadata only. Production is migration-driven, so none of them exist there: the
test schema is built from metadata and enforced the constraints, while the
database the code actually runs against did not.

Only two of the four can be enforced today, so only those two are here:

  A6  receipt(visual_id)                                0 duplicates, 0 nulls
  A1  school_capacity(school, semester, department)     0 duplicates

preUp() re-runs the duplicate scan against the live database and aborts before
any DDL if it is not clean. MySQL applies DDL non-transactionally, so a
duplicate found while creating the second index would otherwise leave the
database half-migrated; refusing to start is the only state that is safe to
retry. The production scan is a reading of a backup, not a promise about the
database this runs on.

Verified against MariaDB 11.4 with the row shapes the production scan found:
the guard aborts and creates nothing while a duplicate exists, ignores NULL key
parts (which a unique index treats as distinct), applies both indexes once
clean, rejects a real duplicate, still accepts the same school and semester in
another department, and down() removes both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sible

phpunit.xml.dist enumerated test directories one by one, so tests/App/ -- 30
files, 64 tests, written for the guard-parity batch -- belonged to no testsuite
and `composer test` never discovered them. CI runs `composer test`, so reverting
any of the code those tests cover broke nothing.

The default suite is now declared by exclusion: every test under tests/ that a
slower, more specific suite does not claim. Suite membership becomes a function
of the filesystem, so a new directory cannot be orphaned by forgetting to list
it. Discovery goes from 1096 to 1167 tests.

PhpUnitSuiteCoverageTest guards the invariant directly, for the case where
someone reverts to enumeration: it fails naming any *Test.php that no suite
covers, and any declared directory that does not exist. Checked against the old
config, where it fails and names all 30 orphaned files.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Catches a mapping that no longer describes anything real -- an index naming a
column that does not exist, a relation that does not resolve. Runs without a
database.

It deliberately does not claim to be the metadata-vs-migrations gate. That gate
is `doctrine:migrations:migrate` against an empty MySQL service followed by an
empty `doctrine:migrations:diff`, and it cannot run today: 49 files in
migrations/ import Doctrine Migrations 1.x's `Doctrine\DBAL\Migrations\
AbstractMigration`, which does not exist in the installed 3.x, so every
`doctrine:migrations:*` command fatals before listing anything. The step
comment records that so the check is not mistaken for more than it is.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant