refactor(git): haal de werkbranch-keuze uit de state-laag, met de tests die ze nooit had (#518)

brenno commented

2026-07-22 19:36:37 +00:00

Owner

Een eerste, afgeronde hap uit #518. Bewust klein: die issue zegt zelf dat dit de duurste tak is om te rebasen, en deze repo werd vandaag door meerdere sessies tegelijk bewerkt — main verschoof zes keer terwijl ik eraan werkte. Eén verplaatsing per PR is hier goedkoper dan één grote.

Wat er verandert

_workBranchFor verhuist van TabsNotifier naar lib/services/git/work_branch.dart.

Het is ontwerpbesluit D3 — bewerken gebeurt op een werkbranch, nooit rechtstreeks op de standaardbranch — en dat is een opslagbesluit. De functie is bovendien zuiver: geen state, geen forge, geen netwerk, alleen een keuze uit haar argumenten.

Dat het in de state-laag zat had één zichtbaar gevolg: er stond geen enkele test op, want hij was alleen te bereiken via een volledige opslag met een forge eromheen. Nu acht tests, en drie daarvan gaan over de beslissing die het makkelijkst over het hoofd wordt gezien.

De subtiele helft

workBranchFor beslist twee dingen tegelijk: waar dit werk landt, én wat de commit is waar de guard straks tegenaan botst. Een lege baseSha is daarbij een uitspraak — "er is geen voorouder" — en niet een lege string uit slordigheid: de aanroeper weigert dan de opslag in plaats van er blind overheen te schrijven.

Dat gedrag was ongetoetst. De tests dekken nu ook de twee gevallen waarin de herkomst er wél is maar niet over dít deck gaat: een ander deck in dezelfde repo, en hetzelfde pad in een andere repo. In beide gevallen is andermans sha geen voorouder, en een guard die daarop draait gaat nergens over.

De teruggave is een typedef WorkBranchChoice geworden in plaats van een anoniem record. De vier velden horen bij elkaar: wie workBranch gebruikt zonder forkFrom takt niet af, en wie baseSha gebruikt zonder midRound weet niet of die basis ergens tegenaan kán botsen.

Ratchet

TabsNotifier zakt van 2379 naar 2325 regels; de basislijn zakt mee. Een ratchet die na een opruiming blijft staan is geen ratchet — dan past het volgende stuk state-logica er zo weer in.

Bijvangst

De SOURCE_MAP-regel van deck_repo_serializer.dart zei nog dat de notities-sidecar door niets in services/git/ geschreven wordt. Dat is sinds #541 onwaar. De registratiepoort toetst aanwezigheid en niet juistheid, dus zulke regels verouderen stil — vandaar dat hij hier meteen recht is gezet, inclusief mirrorDeckFiles, repoUserNotesState en resolveRepoDeckMerge.

Wat van #518 overblijft

Meer git-orkestratie uit lib/state/: saveToGit (164 regels) en _mergeOnConflict (143) zijn de volgende kandidaten, en dat zijn echte verplaatsingen met gedrag eraan — die verdienen hun eigen PR met hun eigen tests.
app_shell blijft groot; het per-klasse-plafond bewaakt het inmiddels wel.
De commentaartaalregel staat in CONTRIBUTING maar heeft nog geen poort. Dat is een aparte, kleine klus.

Wat al gedaan bleek toen ik #518 tegen de code toetste: het per-klasse-plafond bestaat (maxClassLines + classSizeBaseline), de SOURCE_MAP-claim is versmald en afgedwongen (source_map_coverage_test.dart), en de commentaartaalregel staat in CONTRIBUTING. Die drie hoeven niet meer.

Getoetst

make check groen, ook na rebase op de huidige main.
Acht nieuwe tests op een functie die er nul had.

Niet gedraaid: make check-secrets en make sast — gitleaks, trufflehog en semgrep staan geen van drieën op deze machine. Deze wijziging verplaatst code en voegt geen sleutel, netwerkpad of afhankelijkheid toe, maar de eis is daarmee niet gehaald.

Werkt aan #518; die blijft open voor de rest.

Een eerste, afgeronde hap uit #518. Bewust klein: die issue zegt zelf dat dit de duurste tak is om te rebasen, en deze repo werd vandaag door meerdere sessies tegelijk bewerkt — main verschoof zes keer terwijl ik eraan werkte. Eén verplaatsing per PR is hier goedkoper dan één grote. ## Wat er verandert `_workBranchFor` verhuist van `TabsNotifier` naar `lib/services/git/work_branch.dart`. Het is ontwerpbesluit D3 — bewerken gebeurt op een werkbranch, nooit rechtstreeks op de standaardbranch — en dat is een opslagbesluit. De functie is bovendien zuiver: geen state, geen forge, geen netwerk, alleen een keuze uit haar argumenten. Dat het in de state-laag zat had één zichtbaar gevolg: **er stond geen enkele test op**, want hij was alleen te bereiken via een volledige opslag met een forge eromheen. Nu acht tests, en drie daarvan gaan over de beslissing die het makkelijkst over het hoofd wordt gezien. ## De subtiele helft `workBranchFor` beslist twee dingen tegelijk: waar dit werk landt, én wat de commit is waar de guard straks tegenaan botst. Een lege `baseSha` is daarbij een *uitspraak* — "er is geen voorouder" — en niet een lege string uit slordigheid: de aanroeper weigert dan de opslag in plaats van er blind overheen te schrijven. Dat gedrag was ongetoetst. De tests dekken nu ook de twee gevallen waarin de herkomst er wél is maar niet over dít deck gaat: een ander deck in dezelfde repo, en hetzelfde pad in een andere repo. In beide gevallen is andermans sha geen voorouder, en een guard die daarop draait gaat nergens over. De teruggave is een `typedef WorkBranchChoice` geworden in plaats van een anoniem record. De vier velden horen bij elkaar: wie `workBranch` gebruikt zonder `forkFrom` takt niet af, en wie `baseSha` gebruikt zonder `midRound` weet niet of die basis ergens tegenaan kán botsen. ## Ratchet `TabsNotifier` zakt van 2379 naar 2325 regels; de basislijn zakt mee. Een ratchet die na een opruiming blijft staan is geen ratchet — dan past het volgende stuk state-logica er zo weer in. ## Bijvangst De SOURCE_MAP-regel van `deck_repo_serializer.dart` zei nog dat de notities-sidecar door niets in `services/git/` geschreven wordt. Dat is sinds #541 onwaar. De registratiepoort toetst aanwezigheid en niet juistheid, dus zulke regels verouderen stil — vandaar dat hij hier meteen recht is gezet, inclusief `mirrorDeckFiles`, `repoUserNotesState` en `resolveRepoDeckMerge`. ## Wat van #518 overblijft - **Meer git-orkestratie uit `lib/state/`**: `saveToGit` (164 regels) en `_mergeOnConflict` (143) zijn de volgende kandidaten, en dat zijn echte verplaatsingen met gedrag eraan — die verdienen hun eigen PR met hun eigen tests. - **`app_shell`** blijft groot; het per-klasse-plafond bewaakt het inmiddels wel. - **De commentaartaalregel** staat in CONTRIBUTING maar heeft nog geen poort. Dat is een aparte, kleine klus. Wat al gedaan bleek toen ik #518 tegen de code toetste: het per-klasse-plafond bestaat (`maxClassLines` + `classSizeBaseline`), de SOURCE_MAP-claim is versmald *en* afgedwongen (`source_map_coverage_test.dart`), en de commentaartaalregel staat in CONTRIBUTING. Die drie hoeven niet meer. ## Getoetst - `make check` groen, ook na rebase op de huidige main. - Acht nieuwe tests op een functie die er nul had. **Niet gedraaid:** `make check-secrets` en `make sast` — gitleaks, trufflehog en semgrep staan geen van drieën op deze machine. Deze wijziging verplaatst code en voegt geen sleutel, netwerkpad of afhankelijkheid toe, maar de eis is daarmee niet gehaald. Werkt aan #518; die blijft open voor de rest.

brenno added 1 commit

2026-07-22 19:36:37 +00:00

refactor(git): haal de werkbranch-keuze uit de state-laag (#518 )

CI / Gate (Linux) · Format · Analyze · Coverage (push) Failing after 23s

Details

CI / Web hardening (push) Failing after 27s

Details

CI / Docs links (push) Failing after 27s

Details

CI / Gate (Linux) · Format · Analyze · Coverage (pull_request) Failing after 25s

Details

CI / Supply-chain (Trivy · advisory) (push) Failing after 28s

Details

CI / Web hardening (pull_request) Failing after 26s

Details

CI / Docs links (pull_request) Failing after 26s

Details

CI / Supply-chain (Trivy · advisory) (pull_request) Failing after 26s

Details

CI / Test (macos-latest) (push) Has been cancelled

Details

CI / Test (windows-latest) (push) Has been cancelled

Details

CI / Test (macos-latest) (pull_request) Has been cancelled

Details

CI / Test (windows-latest) (pull_request) Has been cancelled

Details

89df3e39c4

D3 — bewerken gebeurt op een werkbranch, nooit rechtstreeks op de
standaardbranch — is een opslagbesluit, en zat als privémethode in
`TabsNotifier`. Het is een zuivere functie: geen state, geen forge, geen
netwerk. Dat het daar zat had één zichtbaar gevolg — er stond geen enkele test
op, want hij was alleen te bereiken via een volledige opslag met een forge
eromheen.

Nu `lib/services/git/work_branch.dart`, met acht tests. De teruggave is een
`typedef` geworden in plaats van een anoniem record: de vier velden horen bij
elkaar, en wie `baseSha` gebruikt zonder `midRound` weet niet of die basis
ergens tegenaan kán botsen.

Die tweede beslissing is de subtiele en was ongetoetst: een lege `baseSha`
betekent "er is geen voorouder" en is iets anders dan een lege string uit
slordigheid — de aanroeper weigert dan de opslag in plaats van er blind
overheen te schrijven. Drie van de acht tests gaan daarover, waaronder het geval
waarin de herkomst over een ánder deck of een andere repo gaat.

`TabsNotifier` zakt van 2379 naar 2325; de basislijn zakt mee, want een ratchet
die na een opruiming blijft staan is geen ratchet.

Onderweg de SOURCE_MAP-regel van `deck_repo_serializer.dart` rechtgezet: die
zei nog dat de notities-sidecar door niets in `services/git/` geschreven wordt,
wat sinds #541 onwaar is. De registratiepoort toetst aanwezigheid, niet
juistheid — dus dat soort regels verouderen stil.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

brenno merged commit 1a59dbf6e0 into main

2026-07-22 19:36:44 +00:00

brenno referenced this pull request from a commit

2026-07-22 19:36:45 +00:00

Merge pull request 'refactor(git): haal de werkbranch-keuze uit de state-laag, met de tests die ze nooit had (#518)' (#674) from refactor/git-uit-state into main

brenno referenced this pull request

2026-07-22 19:37:08 +00:00

[Feature] Structural debt: layering, class size, SOURCE_MAP #518

brenno referenced this pull request

2026-07-22 20:05:48 +00:00

refactor(git): guard-basis en merge-na-conflict uit de state-laag, met de scanners eindelijk gedraaid (#518) #679