Phoebe: finish reconcile observability (loud-log half) + real index EXPLAIN#19
Merged
Merged
Conversation
…XPLAIN Completes the ratified reconcile observability contract (option (c)) on the LOG side; the exit-code half (exitCode() in cmd/rater) was already done. FIX 1 — thread windowExplicit into Rater.Run and gate the reconcile-delete log severity on it. A reconcile-DELETE of a previously-billed rated_usage row is now LOUD on a ROUTINE run (default trailing-hours window, !windowExplicit) — ERROR, because rewriting a prior bill with no operator behind it means events vanished from billing_event (data loss) or an upstream regression dropped them — and QUIET on an EXPLICIT backfill (--since/--until) — INFO, intended convergence. This mirrors the exit-code gate; the reconcile SEMANTICS are unchanged. All Run call sites updated to pass windowExplicit (routine cron path passes false). FIX 2 — test the loud-log half. TestRater_RoutineReconcileDeleteLogsError pins that a routine reconcile-delete (no other anomaly, HasAnomaly stays false) emits an ERROR line; TestRater_BackfillReconcileDeleteLogsInfoNoError pins that the same delete under an explicit backfill logs INFO and NO ERROR. Both capture the INFO and ERROR streams into buffers. The routine-ERROR assertion was demonstrated RED against the pre-fix always-INFO code. Fixed the over-claiming docstring on TestRater_RoutineRunReconcileDeleteExitsNonzero to state it pins ONLY the exit code, now that the log half is separately tested. FIX 3 — make the reconcile-DELETE index EXPLAIN test a real proof. It previously EXPLAINed a standalone `SELECT 1` against an empty rated_usage, where the planner seqscans regardless. Now it populates rated_usage (50 auths x 200 hours = 10k rows), ANALYZEs, and EXPLAINs the ACTUAL reconcile DELETE (the `deleted` CTE shape: window_start range + NOT EXISTS anti-join), asserting the plan chooses rated_usage_window_start_ix at DEFAULT cost (seqscan enabled). Verified discriminating: dropping the index makes the plan fall back to the far costlier auth-leading composite (~280 vs ~8), so the assertion fails without the index. Gate: go build, go vet (+integration), golangci-lint v1.64.8, gofmt all clean; go test -race ./... and -tags=integration -race ./... against live PG all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Stacked on
yaml-rater-fixes-4. Finishes the LOG side of the ratified reconcile observability contract (option (c)); the exit-code side was already done inexitCode().Contracts
Reconcile-delete log-severity contract is now FULLY implemented, matching the exit-code half:
!windowExplicit)--since/--until,windowExplicit)The reconcile SEMANTICS are unchanged either way ("what the latest run says is what bills" —
store.goalways reconciles). Only the log severity (here) and the exit code (cmd/rater, already shipped) turn onwindowExplicit. The flag is threaded intoRater.Runrather than recomputed, because onlycmd/raterknows how the window was chosen — mirroring the exit-code gate and keeping rating observability in the rating package.Fixes
FIX 1 —
Rater.RuntakeswindowExplicit; reconcile-delete logs ERROR on routine, INFO on backfill. A routine run rewriting a prior bill with no operator behind it means events vanished frombilling_event(data loss) or an upstream regression dropped them — so it is ERROR (page). An explicit backfill that deletes the same row is intended convergence — INFO. Deletion count + window appear in both. AllRuncall sites updated (routine cron path passesfalse).FIX 2 — test the loud-log half.
TestRater_RoutineReconcileDeleteLogsError— a routine reconcile-delete with NO other anomaly (HasAnomaly()stays false) emits an ERROR line. Captures the ERROR/INFO streams into buffers. Demonstrated RED against the pre-fix always-INFO code (empty ERROR stream → assertion fails).TestRater_BackfillReconcileDeleteLogsInfoNoError— the same delete under an explicit backfill logs INFO and emits NO ERROR.TestRater_RoutineRunReconcileDeleteExitsNonzero(it pins ONLY the exit code; the log half is now separately tested).FIX 3 — make the reconcile-DELETE index EXPLAIN a real proof. It previously EXPLAINed a standalone
SELECT 1against an EMPTYrated_usage, where the planner seqscans regardless — vacuous. Now it populatesrated_usage(50 auths × 200 hours = 10k rows),ANALYZEs, and EXPLAINs the ACTUAL reconcile DELETE (thedeletedCTE shape:window_startrange +NOT EXISTSanti-join), asserting the plan choosesrated_usage_window_start_ixat DEFAULT cost (seqscan enabled). Verified discriminating: dropping the index makes the plan fall back to the far costlier auth-leading composite (~280 vs ~8 cost), so the assertion fails without the index — not vacuous. A seqscan-off pass follows as belt-and-braces.Gate
go build ./...,go vet ./...(+-tags=integration), golangci-lint v1.64.8,gofmt -l .— all clean.go test -race ./...andPHOEBE_TEST_DATABASE_URL=... go test -tags=integration -race ./...(live PG 16) — all pass, including the e2e suite and the rebuilt index EXPLAIN test (ran, not skipped).🤖 Generated with Claude Code