feat(config): add typed source schema with config lint/fmt/migrate#76
Merged
Conversation
glabs ignores unknown configuration keys silently, so a typo looks exactly like
a setting that works. Add `glabs config lint` to say so out loud, plus `fmt` and
`migrate` — all three built on a typed model of the course file.
Running lint over the real course files finds two classes of dead config that
have been there all along:
- `clone.clone` in every single course file. There is no such option; merely
configuring `clone:` is what enables cloning.
- `vss.blatt2.release.mergeRequest.dockerImages`. The six images are nested
under mergeRequest, but config/release.go:47 reads release.dockerImages, so
none of them are applied.
Neither is fixed here. Both are frozen by the goldens and belong in their own
commits, so the migration diff stays attributable.
The schema (config/source.go) models the *source* form — `extends` unresolved,
course-level students unmerged — which is what round-trips YAML <-> Mongo <->
GraphQL. AssignmentConfig stays what it is: the resolved result. Three tag sets,
one per direction. Pointer fields mark "absent" exactly where the current loader
calls IsSet or checks a map lookup's ok, and nowhere else.
Decoding goes bytes -> map (yaml.v3) -> struct (mapstructure) rather than
straight through yaml.v3, because viper matches keys case-insensitively and real
configs rely on it: `frombranch` and `fromBranch` both work today. yaml.v3
matches tags exactly and would have silently dropped such keys. mapstructure
folds case, which is viper's own behaviour — viper decodes with mapstructure
internally.
Assignments are decoded one at a time. mapstructure's Metadata cannot name a key
inside a ",remain" map (it reports `[<interface {} Value>].clone.clone`), and
lint is only useful if it can say which assignment carries the stray key.
Purely additive: resolution still goes through viper, and the goldens are
untouched. The riskier half — replacing that path with a pure Resolve() over
this schema — is a separate commit with the goldens as its proof.
Two tests carry the weight. TestSchemaClaimsEveryKeyInRealFixtures is the
completeness guard: without it the round-trip test would be near-vacuous, since
a field the decoder never knew about is absent on both sides and passes. And the
round trip is asserted on meaning, not bytes — a struct has no comments and no
key order.
One deliberate divergence, documented in TestDecodeAcceptsAliasViperMissed: the
typed decoder accepts `approvalsRequired`, which viper silently ignores because
it lowercases the key before the alias table compares it against the camelCase
spelling. Fix that in the viper path first, in its own commit, or the resolver
swap stops being a zero-diff proof.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Coverage |
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
1 participant
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.
Schritt 2a des glabs-web-Plans. Rein additiv — der bestehende viper-Auflösungspfad ist unangetastet, die Goldens sind unverändert.
Warum jetzt schon Nutzen entsteht
glabs ignoriert unbekannte Config-Keys stillschweigend. Ein Tippfehler sieht damit exakt aus wie eine Einstellung, die wirkt.
glabs config lintsagt es laut — und findet in Deinen echten Kursdateien zwei Klassen toter Config, die seit jeher drinstehen:clone.clonesteckt in jeder einzelnen Kursdatei (fundc 8×, vss 3×, algdati 2×, mpd, fun). Die sechs Docker-Images invss/blatt2werden komplett ignoriert.Keiner der beiden wird hier gefixt. Beide sind von den Goldens eingefroren und gehören in eigene Commits, damit der Migrationsdiff zuordenbar bleibt.
Dazu
fmt(kanonische Form) undmigrate(Legacy-Schreibweisen hochziehen). Alle drei brauchenResolve()nicht — genau deshalb kommt 2a ohne Risiko aus.Der Schema-Gedanke
config/source.gomodelliert die Quellform:extendsunaufgelöst, Course-Level-Students ungemerged. Das ist, was YAML ⇄ Mongo ⇄ GraphQL round-trippt.AssignmentConfigbleibt, was es ist: das aufgelöste Ergebnis. Drei Tag-Sets, eines pro Richtung. Pointer-Felder markieren „abwesend" exakt dort, wo der heutige LoaderIsSetaufruft oder einokaus einem Map-Lookup prüft — und sonst nirgends.Decode geht
bytes → map(yaml.v3) →struct(mapstructure), nicht direkt über yaml.v3. Grund: viper matcht Keys case-insensitiv und echte Configs verlassen sich darauf (frombranchundfromBranchfunktionieren heute beide). yaml.v3 matcht Tags exakt und hätte solche Keys still verschluckt. mapstructure foldet Case — das ist viper's Verhalten, viper dekodiert intern mit mapstructure.Assignments werden einzeln dekodiert. mapstructures
Metadatakann einen Key innerhalb einer,remain-Map nicht benennen (es meldet[<interface {} Value>].clone.clone), und lint ist nur brauchbar, wenn es sagen kann, welches Assignment den Streu-Key trägt.Zwei Tests tragen die Beweislast
TestSchemaClaimsEveryKeyInRealFixturesist die Vollständigkeitsgarantie. Ohne sie wäre der Round-Trip-Test fast wertlos: ein Feld, das der Decoder gar nicht kennt, ist auf beiden Seiten abwesend und besteht den Test. Der Test nutzt mapstructuresUnused— jeder Key, den kein Feld beansprucht, taucht dort auf. Ergebnis: das Schema beansprucht jeden Key in allen sieben Fixtures, bis auf die zwei bekannten Bugs.Der Round-Trip wird auf Bedeutung geprüft, nicht auf Bytes — ein Struct hat keine Kommentare und keine Key-Reihenfolge. Zusätzlich verifiziert:
fmtist auf allen fünf echten Kursdateien bedeutungserhaltend und idempotent.Eine bewusste Abweichung, dokumentiert
TestDecodeAcceptsAliasViperMissed: der typisierte Decoder akzeptiertapprovalsRequired, das viper stillschweigend ignoriert (viper schreibt den Key klein, bevor die Alias-Tabelle ihn gegen die CamelCase-Schreibweise vergleicht → trifft nie).Konsequenz für die Reihenfolge: in 2b würde der Golden
legacy.approvalslistdadurch von 0 auf 1 kippen. Ich schlage vor, beide Bugs vorher im viper-Pfad zu fixen, jeder mit eigenem Commit und eigenem Golden-Diff — dann bleibt der Resolver-Tausch in 2b ein Null-Diff-Beweis. BeimdockerImages-Bug brauche ich dafür Deine Entscheidung: Code an die YAML anpassen oder YAML an den Code?Verifikation
go test ./...grün;gofmt,go vet,golangci-lintsauberglabs config lintgegen die echtenglabs-yaml-Dateien: findet beide Bugklassen, exit=1glabs config fmtgegen alle fünf echten Dateien: bedeutungserhaltend + idempotentdocs/commands.md🤖 Generated with Claude Code