From fe0e49a134c372d4222fcc8b88240436b5c3a2ab Mon Sep 17 00:00:00 2001 From: Jan Novak Date: Thu, 7 May 2026 22:57:30 +0200 Subject: [PATCH] =?UTF-8?q?feat(go):=20M5.4=20=E2=80=94=20parity=20diff=20?= =?UTF-8?q?binary=20+=20make=20parity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds cmd/parity/main.go: a standalone Go binary that GETs /api/version, /api/adults, /api/juniors, /api/payments from both the Python (:5001) and Go (:8080) backends, scrubs an allowlist (render_time.total, build_meta), and prints cmp.Diff for any remaining differences. Exits 0 on full match, 1 on diffs, 2 on fetch/parse errors — CI-friendly for M7.2. - go/cmd/parity/main.go: flags (-py, -go, -route, -timeout), fetch helper, allowlist scrubber (dotted-path aware), exit-code logic. - go/cmd/parity/scrub_test.go: 4 unit tests for the scrubber. - go/go.mod: promote github.com/google/go-cmp to direct dep. - Makefile: parity target + help entry. - Progress tracker: M5.4 ticked; milestone updated to M5 complete. - Plan archived to docs/plans/2026-05-07-2254-m5-4-parity-binary.md. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 8 + Makefile | 6 +- ...-05-03-2349-go-backend-rewrite-progress.md | 6 +- .../2026-05-07-2254-m5-4-parity-binary.md | 152 ++++++++++++++++++ go/cmd/parity/main.go | 129 +++++++++++++++ go/cmd/parity/scrub_test.go | 57 +++++++ go/go.mod | 1 + 7 files changed, 355 insertions(+), 4 deletions(-) create mode 100644 docs/plans/2026-05-07-2254-m5-4-parity-binary.md create mode 100644 go/cmd/parity/main.go create mode 100644 go/cmd/parity/scrub_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b24900..e2764b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ - Why: both backends used `tmp/_cache.json` with the same keys (`attendance_regular`, `attendance_juniors`, `payments_transactions`, `exceptions_dict`) but different shapes — Python caches post-processed view-model tuples, Go caches raw rows. Whichever wrote last poisoned the cache; running both in parallel produced `ValueError: too many values to unpack (expected 2, got 68)` on Python's `/adults` after the Go server populated `attendance_regular_cache.json` with raw CSV rows. - After upgrading: stop the Go server, hit `/flush-cache` on the Python side once (rewrites `tmp/*.json` with correct shapes), then restart `make web-go` — it will use `tmp/go/` going forward. Required for the M5.4 `make parity` workflow which assumes both backends run side-by-side. +## 2026-05-07 22:55 CEST — feat(go): M5.4 — parity diff binary + `make parity` + +- `go/cmd/parity/main.go`: new standalone binary that GETs `/api/version`, `/api/adults`, `/api/juniors`, `/api/payments` from both Python (:5001) and Go (:8080) backends, scrubs an allowlist (`render_time.total`, `build_meta`), and prints `cmp.Diff` for any remaining differences. Exits 0 on full match, 1 on diffs, 2 on fetch/parse errors — CI-friendly for M7.2. +- `go/cmd/parity/scrub_test.go`: 4 unit tests covering top-level delete, nested delete, missing path, and non-map parent. +- `go/go.mod`: `github.com/google/go-cmp` promoted to direct dependency. +- `Makefile`: `parity` target added (`.PHONY`, help, `cd go && go run ./cmd/parity`). +- `docs/plans/2026-05-07-2254-m5-4-parity-binary.md`: plan archived. + ## 2026-05-07 22:37 CEST — feat(py): M5.3 — Python /api/* shadow endpoints - `app.py`: four new JSON routes (`/api/version`, `/api/adults`, `/api/juniors`, `/api/payments`) mirroring the Go `/api/*` handlers; `_unwrap_view_model_for_api()` helper expands pre-serialised JSON strings and renames `month_labels_json` → `month_labels`, `raw_payments_json` → `raw_payments` to match Go wire contract. diff --git a/Makefile b/Makefile index 1f1efe2..1cba052 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help fees match web web-py web-debug web-go go-build go-test go-test-all go-parity go-run go-sync-debug go-lint capture-fixtures image run sync sync-2026 test test-v docs +.PHONY: help fees match web web-py web-debug web-go go-build go-test go-test-all go-parity go-run go-sync-debug go-lint capture-fixtures parity image run sync sync-2026 test test-v docs export PYTHONPATH := scripts:$(PYTHONPATH) VENV := .venv @@ -29,6 +29,7 @@ help: @echo " make go-lint - Run golangci-lint on Go code" @echo " make go-sync-debug [DAYS=N] - Dry-run Go sync with Fio debug logs and txn table (default DAYS=30)" @echo " make capture-fixtures - Regenerate parity fixture corpus from live Python" + @echo " make parity - Diff /api/* between web-py (:5001) and web-go (:8080); both must be running" @echo " make image - Build Python OCI container image" @echo " make run - Run the built Python Docker image locally" @echo " make sync - Sync Fio transactions to Google Sheets" @@ -102,6 +103,9 @@ go-lint: web-go: go-build ./$(GO_BIN) server +parity: + cd $(GO_SRC) && go run ./cmd/parity $(ARGS) + image: docker build -t fuj-management:latest \ --build-arg GIT_TAG=$$(git describe --tags --always 2>/dev/null || echo "untagged") \ diff --git a/docs/plans/2026-05-03-2349-go-backend-rewrite-progress.md b/docs/plans/2026-05-03-2349-go-backend-rewrite-progress.md index b97f69f..c231e42 100644 --- a/docs/plans/2026-05-03-2349-go-backend-rewrite-progress.md +++ b/docs/plans/2026-05-03-2349-go-backend-rewrite-progress.md @@ -2,9 +2,9 @@ Companion to [2026-05-03-2349-go-backend-rewrite.md](2026-05-03-2349-go-backend-rewrite.md). -**Current milestone:** M4 — IO layer behind interfaces ✅ +**Current milestone:** M5 — JSON-only `/api/...` routes ✅ **Started:** 2026-05-04 -**Last updated:** 2026-05-07 +**Last updated:** 2026-05-07 (M5.4) ## How to use @@ -100,7 +100,7 @@ Goal: byte-equal JSON between Python and Go for every route. This is the parity - [x] **M5.1** Hand-author Go structs for `/api/adults`, `/api/juniors`, `/api/payments`, `/api/version` with explicit `json:` tags matching Python keys; emit JSON Schemas via `github.com/invopop/jsonschema` to `tests/fixtures/api-schema/` — `f253e3f` - [x] **M5.2** Implement Go handlers for `/api/*` routes composing `services/*` results into the JSON structs — `7d48e8f` - [x] **M5.3** Add Python `/api/X` shadow endpoints in [app.py](app.py): `jsonify(view_model_dict)` — no transformation — `40e4a9e` -- [ ] **M5.4** Build `cmd/parity/main.go`: hits both backends' `/api/X`, normalizes allowlist (`render_time.total`, `build_meta`), prints `cmp.Diff`. Add `make parity` target +- [x] **M5.4** Build `cmd/parity/main.go`: hits both backends' `/api/X`, normalizes allowlist (`render_time.total`, `build_meta`), prints `cmp.Diff`. Add `make parity` target **Gate:** For each route, `make parity` reports zero non-allowlisted diffs across the M3 fixture corpus. diff --git a/docs/plans/2026-05-07-2254-m5-4-parity-binary.md b/docs/plans/2026-05-07-2254-m5-4-parity-binary.md new file mode 100644 index 0000000..140b155 --- /dev/null +++ b/docs/plans/2026-05-07-2254-m5-4-parity-binary.md @@ -0,0 +1,152 @@ +# M5.4 — Parity diff binary (`cmd/parity`) + `make parity` + +## Context + +Per [docs/plans/2026-05-03-2349-go-backend-rewrite-progress.md:103](docs/plans/2026-05-03-2349-go-backend-rewrite-progress.md#L103), M5.4 is the next milestone in the Go rewrite. M5.1–M5.3 already landed: + +- M5.1: hand-authored Go response structs at [go/internal/web/api/](go/internal/web/api/) + JSON Schemas in [go/tests/fixtures/api-schema/](go/tests/fixtures/api-schema/). +- M5.2: Go `/api/version|adults|juniors|payments` handlers in [go/internal/web/api/handler.go](go/internal/web/api/handler.go). +- M5.3: Python shadow endpoints in [app.py:157-224](app.py#L157-L224) using `_unwrap_view_model_for_api` for the same JSON shape. + +What's missing: a tool that proves the two backends actually agree on the wire. The M5 gate says "byte-equal JSON between Python and Go for every route." Without a diffing tool, drift between the two implementations slips in silently and we can't gate further milestones (M6 frontend, M7 watch period) on parity. + +This task delivers the **parity contract enforcer**: a Go binary that fetches `/api/*` from both backends, scrubs an allowlist of expected diffs, and prints `cmp.Diff` for everything else. Both backends read the same live Google Sheets — that shared state is what makes parity meaningful, no scenario fixtures needed. + +## Scope + +In scope: +- New binary `go/cmd/parity/main.go` plus a small support package for the allowlist scrubber. +- A unit test for the scrubber. +- New `make parity` target. +- `go-cmp` promoted to a direct dependency. + +Out of scope (explicitly): +- CI integration / nightly job — that's M7.2. +- Fixture-driven offline parity — pure-fn parity already runs via `make go-parity` (M3). +- Hooking `make parity` into `make go-test-all` — leave it manual since it requires two live servers. + +## Approach + +### 1. New binary: `go/cmd/parity/main.go` + +Standalone binary (mirrors task spec — not a subcommand of `fuj`). Stdlib `flag`, no third-party CLI lib. + +**Flags:** +- `-py` — Python base URL, default `http://localhost:5001` +- `-go` — Go base URL, default `http://localhost:8080` +- `-route` — optional single route to diff (e.g. `/api/adults`); empty means iterate all four +- `-timeout` — per-request timeout, default `30s` (sheet fetches are slow on cold cache) + +**Routes**, hard-coded: +```go +var routes = []string{"/api/version", "/api/adults", "/api/juniors", "/api/payments"} +``` +All `GET`, no query params (verified in [app.py:157-224](app.py#L157-L224) and [go/internal/web/api/handler.go:27-68](go/internal/web/api/handler.go#L27-L68)). + +**Per route, the binary:** +1. `GET py+route` and `GET go+route` (sequential — keep it simple; total wall time ~1–2s on warm cache). +2. Verify both return HTTP 200; on non-200, print body and mark route as ERROR. +3. Decode each body into `map[string]any` (NOT into `api.AdultsResponse` — using the typed struct would silently drop unknown Python-side keys, defeating the diff. Map gives `cmp.Diff` clean dotted-path field names for free.). +4. Run `scrub(m, allowlist)` on both decoded maps. +5. `diff := cmp.Diff(pyMap, goMap, cmpopts.EquateEmpty())`. +6. If `diff != ""`, print `=== /api/X ===` header followed by the diff; track for exit code. + +**Exit codes:** +- `0` — all routes match (or the only diffs were under the allowlist). +- `1` — at least one route had a non-allowlisted diff. +- `2` — at least one route failed to fetch / parse (HTTP error, timeout, non-JSON body). + +This makes the binary CI-friendly when M7.2 lands. + +**Output**: human-readable to stdout — header per route, then "OK" or the diff. Final summary line: `parity: 4/4 routes match` or `parity: 2/4 match, 1 diff, 1 error`. + +### 2. Allowlist scrubber + +Live in same package (`main`). Keep it tiny — no need for a separate sub-package. + +```go +var defaultAllowlist = []string{"render_time.total", "build_meta"} + +// scrub walks m and deletes any key whose dotted path matches an allowlist entry. +// "render_time.total" deletes m["render_time"]["total"] only (preserves render_time.breakdown). +// "build_meta" (no dot) deletes m["build_meta"] entirely. +func scrub(m map[string]any, paths []string) { ... } +``` + +Today these fields **don't appear in the JSON** — they're Jinja template-only ([app.py:91-110](app.py#L91-L110)) and the Go side only logs render time via [go/internal/web/middleware/timer.go](go/internal/web/middleware/timer.go). So today the scrub is a no-op. The implementation is forward-compatible insurance: if someone later adds either field to a JSON response on one side only, the parity binary already tolerates it. + +The implementation note in [go/internal/web/middleware/timer.go](go/internal/web/middleware/timer.go) ("the elapsed value maps to render_time.total in the M5 JSON allowlist") confirms this is the intended design. + +### 3. Unit test + +Add `go/cmd/parity/scrub_test.go` with a couple of cases: +- Scrubbing top-level key (`build_meta`) — key removed; siblings untouched. +- Scrubbing nested key (`render_time.total`) — only `total` removed; `breakdown` preserved. +- Path that doesn't exist — no-op, no error. +- Map without the parent key — no-op (don't panic). + +Runs as part of normal `make go-test` (no `-tags` needed). + +### 4. Makefile target + +Add to [Makefile](Makefile): + +```make +parity: + cd $(GO_SRC) && go run ./cmd/parity $(ARGS) +``` + +Add `parity` to the `.PHONY` line at [Makefile:1](Makefile#L1) and a `help` entry like: + +``` +@echo " make parity - Diff /api/* between web-py (:5001) and web-go (:8080); both must be running" +``` + +Don't depend on `go-build` — `go run` compiles ad-hoc, and parity is interactive enough that the slight rebuild cost doesn't matter. + +### 5. Dependency + +`github.com/google/go-cmp v0.7.0` is already a transitive dep ([go/go.sum:24-25](go/go.sum#L24-L25)) but not in `go.mod`'s `require` block. After adding the import, run `go mod tidy` inside `go/` to promote it to direct. No new external deps. + +## Files to add / modify + +**Add:** +- [go/cmd/parity/main.go](go/cmd/parity/main.go) — flags, route loop, fetch+scrub+diff, exit codes +- [go/cmd/parity/scrub_test.go](go/cmd/parity/scrub_test.go) — unit tests for the scrubber + +**Modify:** +- [Makefile](Makefile) — `.PHONY`, help block, `parity` target +- [go/go.mod](go/go.mod) — promote `go-cmp` to direct (via `go mod tidy`) +- [go/go.sum](go/go.sum) — likely unchanged (already pinned) +- [docs/plans/2026-05-03-2349-go-backend-rewrite-progress.md](docs/plans/2026-05-03-2349-go-backend-rewrite-progress.md) — tick M5.4, add commit SHA +- [CHANGELOG.md](CHANGELOG.md) — entry per project convention + +**Per [CLAUDE.md](CLAUDE.md) plans convention:** copy this plan to `docs/plans/2026-05-07-HHMM-m5-4-parity-binary.md` (timestamp via `date "+%Y-%m-%d-%H%M"`) before opening the MR, since plan files are committed for posterity. + +## Existing utilities to reuse + +- `cmp.Diff` + `cmpopts.EquateEmpty()` from `github.com/google/go-cmp/cmp` (already in go.sum). +- Stdlib `net/http`, `encoding/json`, `flag` — sufficient; no need to pull a CLI framework. +- No existing scrubber to reuse — the one in [scripts/scrub_fixtures.py](scripts/scrub_fixtures.py) operates on the *capture* path and renames PII; the parity scrubber is a different concern (path-based deletion of expected diffs). + +## Verification + +Manual smoke test (the binary is meant to be run against live servers): + +1. **Sanity / build:** `cd go && go build ./cmd/parity && go test ./cmd/parity/...` — compiles + scrubber unit test passes. +2. **Both backends up:** + - Terminal A: `make web-py` + - Terminal B: `make web-go` + - Terminal C: `make parity` + Expected: `parity: 4/4 routes match`, exit 0. +3. **Negative test:** add a literal extra field to one Go handler temporarily (e.g. `"diagnostic": "test"` in `VersionResponse`), rebuild, re-run `make parity`. Expected: non-zero exit, diff shown for `/api/version`. Revert. +4. **Allowlist test:** in a unit test (or by manually constructing a payload with `render_time.total` injected), confirm the scrubber removes it before the diff stage. +5. **CHANGELOG** entry added; progress tracker ticked with the merge commit SHA. + +Per [CLAUDE.md](CLAUDE.md) branching policy: this is a feature, so work happens on `feat/go-m5-4-parity-binary`, push with `-u`, open MR with `tea pr create --base main --head feat/go-m5-4-parity-binary`. User merges in Gitea. + +## Notes & risks + +- **Cache-warmth dependency:** Both backends cache aggressively. A cold-cache fetch on Python can take 30s+ — hence the configurable timeout. If parity is run immediately after `flush-cache`, expect the first run to be slow. +- **Order-sensitive lists:** `cmp.Diff` on `map[string]any` treats slices as ordered. If either backend ever returns members/transactions in a different order, the diff will flag it — that's a real parity bug, not a false positive, so this is correct behavior. If we hit ordering instability later, fix the source, don't add `cmpopts.SortSlices`. +- **Float formatting:** Both sides go through `encoding/json` (Go) and `jsonify` (Python `json.dumps`). Floats in totals/balances may format differently (`123` vs `123.0`). If this surfaces, the Go side already uses typed structs with `int` where appropriate — investigate at the source rather than allowlisting. diff --git a/go/cmd/parity/main.go b/go/cmd/parity/main.go new file mode 100644 index 0000000..643a9a2 --- /dev/null +++ b/go/cmd/parity/main.go @@ -0,0 +1,129 @@ +package main + +import ( + "encoding/json" + "flag" + "fmt" + "io" + "net/http" + "os" + "strings" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" +) + +var allRoutes = []string{"/api/version", "/api/adults", "/api/juniors", "/api/payments"} + +// defaultAllowlist holds dotted key paths to strip before diffing. +// These fields are expected to differ between backends (e.g. build tags, timing) +// or may appear on one side only. Today both are absent from the JSON — this is +// forward-compatible insurance for if either is added later. +var defaultAllowlist = []string{"render_time.total", "build_meta"} + +func main() { + pyURL := flag.String("py", "http://localhost:5001", "Python backend base URL") + goURL := flag.String("go", "http://localhost:8080", "Go backend base URL") + route := flag.String("route", "", "single route to diff, e.g. /api/adults (default: all)") + timeout := flag.Duration("timeout", 30*time.Second, "per-request HTTP timeout") + flag.Parse() + + client := &http.Client{Timeout: *timeout} + + targets := allRoutes + if *route != "" { + targets = []string{*route} + } + + matched, diffs, errs := 0, 0, 0 + + for _, r := range targets { + pyMap, err1 := fetch(client, *pyURL+r) + goMap, err2 := fetch(client, *goURL+r) + + if err1 != nil || err2 != nil { + fmt.Printf("=== %s ===\n", r) + if err1 != nil { + fmt.Printf("ERROR (py): %v\n", err1) + } + if err2 != nil { + fmt.Printf("ERROR (go): %v\n", err2) + } + fmt.Println() + errs++ + continue + } + + scrub(pyMap, defaultAllowlist) + scrub(goMap, defaultAllowlist) + + diff := cmp.Diff(pyMap, goMap, cmpopts.EquateEmpty()) + if diff == "" { + fmt.Printf("=== %s ===\nOK\n\n", r) + matched++ + } else { + fmt.Printf("=== %s ===\n%s\n", r, diff) + diffs++ + } + } + + total := len(targets) + fmt.Printf("parity: %d/%d routes match", matched, total) + if diffs > 0 { + fmt.Printf(", %d diff", diffs) + if diffs > 1 { + fmt.Print("s") + } + } + if errs > 0 { + fmt.Printf(", %d error", errs) + if errs > 1 { + fmt.Print("s") + } + } + fmt.Println() + + if errs > 0 { + os.Exit(2) + } + if diffs > 0 { + os.Exit(1) + } +} + +func fetch(client *http.Client, url string) (map[string]any, error) { + resp, err := client.Get(url) //nolint:noctx + if err != nil { + return nil, err + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("read body: %w", err) + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) + } + var m map[string]any + if err := json.Unmarshal(body, &m); err != nil { + return nil, fmt.Errorf("decode JSON: %w", err) + } + return m, nil +} + +// scrub removes keys from m whose dotted paths appear in paths. +// A bare segment (no dot) deletes a top-level key. +// A two-segment path "parent.child" deletes child from m["parent"] if it is a map. +func scrub(m map[string]any, paths []string) { + for _, path := range paths { + parts := strings.SplitN(path, ".", 2) + if len(parts) == 1 { + delete(m, parts[0]) + } else { + if child, ok := m[parts[0]].(map[string]any); ok { + delete(child, parts[1]) + } + } + } +} diff --git a/go/cmd/parity/scrub_test.go b/go/cmd/parity/scrub_test.go new file mode 100644 index 0000000..ea2efdc --- /dev/null +++ b/go/cmd/parity/scrub_test.go @@ -0,0 +1,57 @@ +package main + +import "testing" + +func TestScrubTopLevel(t *testing.T) { + m := map[string]any{ + "build_meta": map[string]any{"tag": "v1"}, + "other": "keep", + } + scrub(m, []string{"build_meta"}) + if _, ok := m["build_meta"]; ok { + t.Error("expected build_meta to be removed") + } + if m["other"] != "keep" { + t.Error("expected other to be preserved") + } +} + +func TestScrubNested(t *testing.T) { + m := map[string]any{ + "render_time": map[string]any{ + "total": "0.123", + "breakdown": "fetch:0.1s", + }, + "other": "keep", + } + scrub(m, []string{"render_time.total"}) + rt, ok := m["render_time"].(map[string]any) + if !ok { + t.Fatal("render_time should still be present") + } + if _, ok := rt["total"]; ok { + t.Error("expected render_time.total to be removed") + } + if rt["breakdown"] != "fetch:0.1s" { + t.Error("expected render_time.breakdown to be preserved") + } + if m["other"] != "keep" { + t.Error("expected other to be preserved") + } +} + +func TestScrubMissingPath(t *testing.T) { + m := map[string]any{"foo": "bar"} + scrub(m, []string{"nonexistent", "render_time.total"}) + if m["foo"] != "bar" { + t.Error("expected foo to be preserved") + } +} + +func TestScrubNestedParentNotMap(t *testing.T) { + m := map[string]any{"render_time": "not-a-map"} + scrub(m, []string{"render_time.total"}) + if m["render_time"] != "not-a-map" { + t.Error("expected render_time to be unchanged when it is not a map") + } +} diff --git a/go/go.mod b/go/go.mod index ccc4b54..73f0f76 100644 --- a/go/go.mod +++ b/go/go.mod @@ -3,6 +3,7 @@ module fuj-management/go go 1.26.1 require ( + github.com/google/go-cmp v0.7.0 github.com/invopop/jsonschema v0.14.0 golang.org/x/net v0.53.0 golang.org/x/text v0.36.0