docs: add pre-feature cleanup task list and API parity plan
Deploy to K8s / deploy (push) Successful in 14s
Deploy to K8s / deploy (push) Successful in 14s
Task list from the 2026-10-08 repo audit, plus the plan for the two parity fixes found while validating the legacy/ move. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,157 @@
|
|||||||
|
# Pre-feature cleanup — task list
|
||||||
|
|
||||||
|
Findings from a full repo audit on 2026-10-08, written up as tasks to clear
|
||||||
|
before new feature work starts. Companion to the Go rewrite tracker,
|
||||||
|
[2026-05-03-2349-go-backend-rewrite-progress.md](2026-05-03-2349-go-backend-rewrite-progress.md).
|
||||||
|
|
||||||
|
## State at audit time
|
||||||
|
|
||||||
|
- Two full implementations live side by side: Flask (`app.py`, `scripts/`,
|
||||||
|
`templates/`) and Go (`go/`). Recent changes were made in both.
|
||||||
|
- Update 2026-10-08: the Python code was moved to `legacy/` after this audit.
|
||||||
|
Python paths below are relative to `legacy/` (`build/Dockerfile` is now
|
||||||
|
`legacy/build/Dockerfile`).
|
||||||
|
- Python tests pass (32 tests). Go unit tests and the parity suite pass.
|
||||||
|
Go lint could not run (see T7).
|
||||||
|
- The gitops workflow bumps the cluster deployment to the `-go` image, so Go
|
||||||
|
appears to be what is deployed. This was not verified against the cluster.
|
||||||
|
- `CLAUDE.md` and all docs outside `docs/plans/` describe a Python-only
|
||||||
|
project. The rewrite tracker stops at M6; M7 and M8 are untouched.
|
||||||
|
|
||||||
|
## How to use
|
||||||
|
|
||||||
|
- Tick a checkbox when the task's PR or commit lands and append the SHA.
|
||||||
|
- Tasks in "Fix first" are small and independent of each other and of the
|
||||||
|
cutover decision. Do them in any order.
|
||||||
|
- T5 is a decision. Several cleanup tasks become unnecessary once it is made;
|
||||||
|
they are marked "moot if Python is retired".
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Fix first
|
||||||
|
|
||||||
|
- [ ] **T1** Remove the secret leak in `.gitea/workflows/kubernetes-deploy.yaml`.
|
||||||
|
- It runs on every push to every branch and deploys nothing.
|
||||||
|
- It echoes the raw Vault login response, the Kanidm token response and the
|
||||||
|
OIDC token response body to the log, and dumps the environment twice.
|
||||||
|
- It uses `--insecure-skip-tls-verify=true`.
|
||||||
|
- Delete or disable the workflow. Rotate the Vault AppRole secret and the
|
||||||
|
Kanidm API token if anyone else can read the CI logs.
|
||||||
|
- [ ] **T2** Confirm the fee rates for 2026-09 onward.
|
||||||
|
- `ADULT_FEE_MONTHLY_RATE` ends at 2026-08 in `scripts/attendance.py` and
|
||||||
|
`go/internal/domain/fees/fees.go`.
|
||||||
|
- September and October 2026 are billed at the defaults: 700 CZK adults,
|
||||||
|
500 CZK juniors.
|
||||||
|
- If those are not the intended rates, add the months in both files.
|
||||||
|
- [ ] **T3** Remove the hardcoded 2026 window from Python `/sync-bank`
|
||||||
|
(`app.py`, the `sync_to_sheets` call with `2026-01-01` / `2026-12-31`).
|
||||||
|
- It stops picking up transactions on 2027-01-01. Go already uses the
|
||||||
|
current year.
|
||||||
|
- Same applies to the `sync-2026` Makefile target.
|
||||||
|
- Moot if Python is retired before then.
|
||||||
|
- [ ] **T4** Remove member names from tracked files (PII rule in `CLAUDE.md`).
|
||||||
|
- `CHANGELOG.md` line 29 names a member in a fix description.
|
||||||
|
- `tests/test_match_payments.py`, `tests/test_reconcile_exceptions.py` and
|
||||||
|
`go/internal/domain/reconcile/reconcile_test.go` use names that may be
|
||||||
|
real. Check them and replace with obviously fictional ones.
|
||||||
|
- Three files in `docs/plans/` contain the same names:
|
||||||
|
`2026-05-05-1717-payment-person-name-canonicalization.md`,
|
||||||
|
`2026-05-06-1626-infer-payments-junior-roster.md`,
|
||||||
|
`2026-05-11-2353-fill-first-multi-month-allocation.md`.
|
||||||
|
- The parity fixtures under `go/tests/fixtures/` are scrubbed and need no
|
||||||
|
change.
|
||||||
|
- Decide whether the names also need removing from git history.
|
||||||
|
|
||||||
|
## Decide before new features
|
||||||
|
|
||||||
|
- [ ] **T5** Decide how to finish the Python → Go cutover (M7 and M8 in the
|
||||||
|
rewrite tracker).
|
||||||
|
- Recommended: declare Go primary now, freeze Python, run one month-end
|
||||||
|
comparison of the balance report on both sides, then delete Python.
|
||||||
|
- Until this is done, every feature has to be written twice.
|
||||||
|
- Confirm first that the cluster really runs the `-go` image.
|
||||||
|
- [ ] **T6** Add a test job to CI.
|
||||||
|
- `.gitea/workflows/build.yaml` only builds images on tag push. Nothing
|
||||||
|
runs tests.
|
||||||
|
- Run `go test -race ./...`, the parity suite (`-tags=parity`) and
|
||||||
|
`golangci-lint` on pull requests.
|
||||||
|
- Stop building the Python image on every tag once Python is retired.
|
||||||
|
- [ ] **T7** Fix `make go-lint`.
|
||||||
|
- `go/.golangci.yml` is in the v1 config format. golangci-lint 2.12 refuses
|
||||||
|
to load it ("unsupported version of the configuration").
|
||||||
|
- Migrate the config to v2 and pin the linter version used in CI.
|
||||||
|
- [ ] **T8** Protect the side-effectful routes.
|
||||||
|
- Neither app has any authentication.
|
||||||
|
- `GET /sync-bank` writes to the payments sheet, so a link prefetcher or
|
||||||
|
crawler can trigger a sync. Change it to POST with a confirmation page,
|
||||||
|
as `/flush-cache` already does.
|
||||||
|
- Confirm that the ingress enforces access control, or add it in the app.
|
||||||
|
- The Go server uses `http.ListenAndServe` with no read, write or idle
|
||||||
|
timeouts (`go/internal/web/server.go`). Use an `http.Server` with
|
||||||
|
timeouts.
|
||||||
|
- [ ] **T9** Move fee rates and merged months out of code.
|
||||||
|
- Today a rate change needs a code change in two languages and a release.
|
||||||
|
- Candidate: a tab in the payments sheet, read the same way as
|
||||||
|
`exceptions`.
|
||||||
|
- This also removes the cause of T2.
|
||||||
|
|
||||||
|
## Cleanup
|
||||||
|
|
||||||
|
- [x] **T10** Rewrite `CLAUDE.md` for the current state: Go commands,
|
||||||
|
architecture and key modules, and which backend is primary. Done with the
|
||||||
|
move of Python to `legacy/`.
|
||||||
|
- [ ] **T11** Collapse the documentation into one set.
|
||||||
|
- `docs/by-claude-opus/`, `docs/by-gemini/` and the top-level docs overlap,
|
||||||
|
date from February–March 2026 and predate the Go rewrite.
|
||||||
|
- Update `docs/_sidebar.md` and `docs/README.md` to match.
|
||||||
|
- [ ] **T12** Update the rewrite tracker.
|
||||||
|
- M6.7 is unchecked although the changelog records it as done.
|
||||||
|
- M7.1 and M8.4 refer to a `docker-compose.yml` that does not exist.
|
||||||
|
- [ ] **T13** Remove leftovers: `.agent/rules.md` (Antigravity config) and
|
||||||
|
`prompts/`.
|
||||||
|
- [ ] **T14** Prune merged branches: 42 local and 44 remote branches are
|
||||||
|
fully merged into `main`.
|
||||||
|
- [ ] **T15** Clean up `.claude/settings.json`.
|
||||||
|
- It is tracked and contains absolute paths under one user's home
|
||||||
|
directory and one-off commands.
|
||||||
|
- Keep only shared, generic permissions there; move the rest to
|
||||||
|
`.claude/settings.local.json`, which is already ignored.
|
||||||
|
- [ ] **T16** Align versions.
|
||||||
|
- `pyproject.toml` says `0.10`; git tags are at `0.39`.
|
||||||
|
- `build/Dockerfile` pip-installs unpinned packages and ignores `uv.lock`.
|
||||||
|
- `go.mod` and `go/build/Dockerfile` are on Go 1.26; the local toolchain is
|
||||||
|
1.27.
|
||||||
|
- Dependencies on both sides are a few months behind.
|
||||||
|
- The Python items are moot if Python is retired.
|
||||||
|
- [ ] **T17** Stop the Python tests from calling live Google APIs.
|
||||||
|
- `warmup_cache()` runs at import of `app.py`, so `make test` makes real
|
||||||
|
Drive and Sheets read calls and needs the credentials file.
|
||||||
|
- Moot if Python is retired.
|
||||||
|
- [ ] **T18** Set the time zone in the Go container.
|
||||||
|
- The image has no time zone configured, so `time.Now()` is UTC and the
|
||||||
|
"current month" flips one to two hours late at month boundaries.
|
||||||
|
- The Python image sets `Europe/Prague`.
|
||||||
|
- [ ] **T19** Decide where `fujarna/` lives.
|
||||||
|
- `fujarna/project.md` is untracked. It is a brief for a separate
|
||||||
|
tournament website and shares almost nothing with this codebase.
|
||||||
|
- Options: its own repository, or a clearly separate Go module here.
|
||||||
|
- [ ] **T20** Add tests for the untested Go packages: `cmd/fuj`,
|
||||||
|
`internal/io/sheets`, `internal/io/drive`, `internal/config` (about 590
|
||||||
|
lines in total).
|
||||||
|
|
||||||
|
## Found during validation of the `legacy/` move
|
||||||
|
|
||||||
|
Plan: [2026-10-08-1103-api-parity-vs-months-to-show.md](2026-10-08-1103-api-parity-vs-months-to-show.md).
|
||||||
|
|
||||||
|
- [x] **T21** Go rendered numeric VS cells in exponent form
|
||||||
|
(`2.609767878e+09`). Sheet text cells now go through `sheets.CellString`.
|
||||||
|
- [x] **T22** Python `/api/adults` and `/api/juniors` did not return
|
||||||
|
`months_to_show`. Backported; `make parity` reports 3/3 routes matching.
|
||||||
|
- [x] **T23** Write VS to the payments sheet as text. Both sync
|
||||||
|
implementations use `USER_ENTERED`, so Sheets stored VS as a number and
|
||||||
|
dropped leading zeros. New rows now carry a text marker; existing rows are
|
||||||
|
left as they are and are read correctly either way.
|
||||||
|
|
||||||
|
## Notes & decisions
|
||||||
|
|
||||||
|
(Add entries as you go. Format: `YYYY-MM-DD — short note`.)
|
||||||
@@ -0,0 +1,112 @@
|
|||||||
|
# Fix the two Go/Python parity differences
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
Running `make parity` against both containers on 2026-10-08 reported
|
||||||
|
`0/3 routes match`. Two causes, neither related to the move to `legacy/`:
|
||||||
|
|
||||||
|
1. **VS rendered in scientific notation by Go.** The payments sheet is read
|
||||||
|
with `UNFORMATTED_VALUE`, so a numeric VS cell arrives as `float64`. Go
|
||||||
|
stringifies cells with `fmt.Sprint`, which prints any float ≥ 1e6 in
|
||||||
|
exponent form (`2.609767878e+09`). Python already converts integer floats
|
||||||
|
to plain digits for VS (`get_str` in `fetch_sheet_data`). VS must always be
|
||||||
|
a plain string.
|
||||||
|
2. **`months_to_show` missing from the Python JSON API.** Go returns it in
|
||||||
|
`/api/adults` and `/api/juniors` (and it is `required` in the committed
|
||||||
|
schemas). Python already has the feature in its HTML pages
|
||||||
|
(`MONTHS_TO_SHOW` is passed to the templates) but never added the key to
|
||||||
|
the API responses. Backport the key so the API contract matches.
|
||||||
|
|
||||||
|
Outcome: `make parity` reports 3/3 routes matching.
|
||||||
|
|
||||||
|
## Branching
|
||||||
|
|
||||||
|
Both are bug fixes that touch `legacy/` paths, so they depend on the
|
||||||
|
uncommitted `feat/python-to-legacy` work. Commit that branch first (still
|
||||||
|
awaiting confirmation), then branch `fix/api-parity-vs-months-to-show` from
|
||||||
|
it, with one commit per task. Open the MR with `tea` after the legacy move is
|
||||||
|
merged, or target it at that branch.
|
||||||
|
|
||||||
|
## Task 1 — VS (and other text cells) always a plain string in Go
|
||||||
|
|
||||||
|
Root cause is `getVal` in `parseTransactionRows`
|
||||||
|
(`go/internal/services/membership/sources.go`, the `fmt.Sprint(row[i])`
|
||||||
|
closure). The same pattern exists in `get` in
|
||||||
|
`go/internal/services/banksync/infer.go`.
|
||||||
|
|
||||||
|
1. Add `CellString(v any) string` to `go/internal/io/sheets` (new small file
|
||||||
|
`cell.go`; both services already import this package):
|
||||||
|
- `nil` → `""`
|
||||||
|
- `float64` → `strconv.FormatFloat(x, 'f', -1, 64)` (never exponent form;
|
||||||
|
`2609767878.0` → `"2609767878"`, `12.5` → `"12.5"`)
|
||||||
|
- `string` → unchanged
|
||||||
|
- anything else → `fmt.Sprint(v)`
|
||||||
|
2. Use it in `getVal` in `sources.go`. This fixes VS and protects the other
|
||||||
|
text columns (Message, Bank ID, Sender, …) from the same failure.
|
||||||
|
- `getVal` currently returns `"<nil>"` for nil cells and the inferred-amount
|
||||||
|
code checks for that literal; simplify that check to `iv != ""`.
|
||||||
|
- Leave `getRaw`/`FormatDate` and `parseFloat` for Date and Amount as they
|
||||||
|
are — those must stay numeric.
|
||||||
|
3. Use it in `get` in `banksync/infer.go` (keeping the `TrimSpace`), so
|
||||||
|
inference sees the same text the dashboard shows.
|
||||||
|
4. Tests:
|
||||||
|
- `go/internal/io/sheets/cell_test.go`: table test for `CellString`
|
||||||
|
(large integer float, small integer float, fractional, string with
|
||||||
|
leading zeros, nil).
|
||||||
|
- Extend `TestLoadTransactions` in
|
||||||
|
`go/internal/services/membership/sources_test.go` with a row whose VS is
|
||||||
|
`2609767878.0` and whose Bank ID is an 11-digit float; assert plain
|
||||||
|
digit strings.
|
||||||
|
5. Python: no code change needed — `get_str` already does this for VS. Add one
|
||||||
|
regression test for `fetch_sheet_data` in
|
||||||
|
`legacy/tests/test_match_payments.py` (mock the Sheets service, numeric VS
|
||||||
|
cell → `"2609767878"`) so the two sides are pinned to the same behaviour.
|
||||||
|
|
||||||
|
Not included (note for later): VS is *written* to the sheet with
|
||||||
|
`USER_ENTERED` by both sync implementations, so Sheets stores it as a number
|
||||||
|
and drops leading zeros. Fixing that means writing VS as text and is a
|
||||||
|
separate change to the sheet's data.
|
||||||
|
|
||||||
|
## Task 2 — backport `months_to_show` to the Python JSON API
|
||||||
|
|
||||||
|
`legacy/app.py` only; `MONTHS_TO_SHOW` is already imported there.
|
||||||
|
|
||||||
|
1. In `api_adults()` and `api_juniors()`, add the key to the response:
|
||||||
|
build `out = _unwrap_view_model_for_api(vm)`, set
|
||||||
|
`out["months_to_show"] = MONTHS_TO_SHOW`, return `jsonify(out)`.
|
||||||
|
- Do not add it inside `build_*_view_model` or `_unwrap_view_model_for_api`:
|
||||||
|
the HTML routes already pass `months_to_show=` to `render_template`
|
||||||
|
alongside `**vm`, and a duplicate key would raise `TypeError`.
|
||||||
|
- `/api/payments` is unchanged; Go does not return the key there.
|
||||||
|
2. Tests in `legacy/tests/test_app.py`:
|
||||||
|
- Add `"months_to_show"` to the expected key list used by
|
||||||
|
`test_api_adults` / `test_api_juniors` (the tuple near the top of the
|
||||||
|
file), and assert the value is an `int`.
|
||||||
|
3. Go: no change. The key and schemas
|
||||||
|
(`go/tests/fixtures/api-schema/{adults,juniors}.schema.json`) already
|
||||||
|
contain it.
|
||||||
|
|
||||||
|
## Wrap-up
|
||||||
|
|
||||||
|
- `CHANGELOG.md` entry at the top (after confirmation it works), per repo
|
||||||
|
convention.
|
||||||
|
- Copy this plan to
|
||||||
|
`docs/plans/2026-10-08-HHMM-api-parity-vs-months-to-show.md`, per the repo's
|
||||||
|
plan-location rule.
|
||||||
|
- Add both items to
|
||||||
|
`docs/plans/2026-10-08-1026-pre-feature-cleanup-tasks.md` as done tasks so
|
||||||
|
the audit list stays complete.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
1. `make go-test-all` — unit tests (including the new ones) and parity
|
||||||
|
fixtures pass.
|
||||||
|
2. `make test` — Python tests pass, including the new API key and VS tests.
|
||||||
|
3. `cd go && go test ./internal/web/api/...` — schema golden test still
|
||||||
|
matches (no schema change expected).
|
||||||
|
4. End to end: start `make web-py` and `make web-go`, then `make parity`;
|
||||||
|
expect `3/3 routes match`. Alternatively repeat the two-container run used
|
||||||
|
to find the bugs (`make image`, Go image from `go/build/Dockerfile`, then
|
||||||
|
`go run ./cmd/parity -py … -go …`).
|
||||||
|
5. Open `/payments` on :8080 and check the affected transaction's VS shows as
|
||||||
|
plain digits.
|
||||||
Reference in New Issue
Block a user