Skip to content

feat(billing-matrix): add billing regression suite - #6323

Open
annvelents wants to merge 10 commits into
mainfrom
billing-matrix
Open

annvelents wants to merge 10 commits into
mainfrom
billing-matrix

Conversation

@annvelents

@annvelents annvelents commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Context

Billing defects often involve combined features or changes made part-way through a cycle. This suite describes those scenarios as YAML setup, dated timeline, and expected billing figures, with arithmetic explaining each expectation. The plain-Ruby runner exercises the real REST API and uses deliberately failing canaries to check its assertion mechanisms.

Manual execution

After merge, open Actions → Billing Matrix → Run workflow. There is no schedule, and the billing scenarios are not added to ordinary PR test runs. Focused unit specs in spec/billing_matrix/ run in normal CI and exercise the harness without launching the billing scenarios. Importing the CLI does not boot its scenario context or clean the database.

  • dry_run defaults to checked. It executes the suite and saves results without publishing a ledger PR or posting to Slack.
  • Unchecking it allows ledger publication and, when configured, Slack reporting. SLACK_BOT_TOKEN and SLACK_DM_USER_ID are optional for execution; missing values skip Slack steps.
  • Every run retains its JSON results and log. Completed runs show row counts and transitions in the Actions summary. Failed runner processes, unproven canaries, and errored scenarios fail the workflow instead of appearing successful. Errored rows reach the summary and configured Slack report before the job fails; incomplete runs do not publish ledger updates.
  • Concurrent manual runs are serialized. Ledger cleanup is persisted even when it produces no new alert.
  • Shard/database pairing is checked before any runner cleanup. File and directory inputs from repeated --rows arguments are combined before validating control references.
  • Finding IDs (pins) are retained in results, ledger entries, and transition reports. Deduplication remains per scenario; separate scenarios can cover the same finding.

The ledger records known failures and reports failure/fix transitions. Changes go through a PR, and reruns use that open PR’s ledger so transitions are reported once before merge. Closed or merged PR branches are ignored as input. Publishing uses a lease to avoid overwriting a concurrent branch update, and cleanup back to the base ledger does not attempt to open an empty PR.

The companion maintainer in https://github.com/getlago/lago-ai-skills/pull/153 also runs by click, proposing coverage updates through draft PRs.

Coverage and review

The corpus contains 22 billing scenarios and five canaries. It includes licence-on/off controls, invoice-setting precedence, draft repricing, paid/granted wallet credits, and preview/finalized invoice differences.

Some rows deliberately assert intended billing behavior that the current implementation violates. Review their arithmetic and product assumptions before treating these mismatches as confirmed defects; expectations should not be changed merely to match observed output.

Start with billing_matrix/README.md and billing_matrix/runner/CONTRACT.md. The PR is reduced from 21,620 to approximately 5,000 added lines, including the focused regression specs, by removing the unused historical archive. Its files remain linked at a pinned Git revision in the README. Only billing_matrix/reference/legality.rb remains as feature-combination guidance for the maintainer; all 27 executable rows and the manual workflow are retained.

Validation

  • RuboCop passes for the remaining billing-matrix Ruby files; Actionlint 1.7.7 passes for the workflow.
  • All 27 rows load and validate without Rails, rechecked after removing the archive. Standalone checks pass for ledger failure/fix deduplication and cleanup, monetary comparison, and canary verdicts.
  • The full matrix run on September 10 used the API container and a dedicated test database: 27 rows, 23 passed (including all five canaries), four expected billing mismatches, zero errors, exit 0. The mismatches are commitment loss on plan override, commitment/progressive-billing interaction, credit-note restoration on void, and progressive-billing credit missing from preview.
  • The real run's ledger report identifies the preview mismatch as new and suppresses repeat alerts for the three existing failures.
  • The harness has 38 regression examples, covered by the passing full GitHub spec run on c9f75f64a. The prior 29-example suite passed in the API container; the latest local attempt was killed during startup, and its retry was stopped after all CI spec shards passed. The specs cover row validation and selection (including unmatched filters, file inputs, and cross-path controls), shard rejection before cleanup, finding-ID propagation, fee ambiguity and numeric comparisons, canary result accounting, ledger transitions, errored-row reporting, and billing-entity tax assignment from the same setup. Three workflow regressions execute the actual shell scripts with local Git remotes, covering pending PR reruns, cleanup, stale closed PR state, and dry runs. RuboCop passes for these changes.
  • Replayed the actual workflow scripts with complete and errored fixture results: both produce a summary; the errored run leaves the ledger untouched and finishes with a failure. No Slack or GitHub publication was performed.
  • Verified that importing the CLI does not boot Rails, and all 27 rows still validate without Rails. All 10 GitHub spec shards, lint, and migrations pass on c9f75f64a.
  • GitHub workflow execution and optional Slack/ledger publication remain to be checked with the first manual run after merge.

annvelents and others added 7 commits September 7, 2026 20:54
## Context

The billing defects that reach customers concentrate in two places:
combined features, where one reducer interacts with another, and
subscriptions changed part-way through a cycle. A previous attempt at
covering these lost its credibility not to missed bugs but to rows that
reported green while asserting nothing — an assertion helper swallowed
every failure, and a five-value axis turned out to measure one behaviour.

## Description

Scenario rows are YAML: a setup, a dated timeline, and the few figures
that matter, each accompanied by the arithmetic that justifies it. A
plain-Ruby runner executes them against the real REST API, so serializer
and API defects stay in range, and it runs on a schedule rather than in
pull-request CI, costing no review time.

Nothing is derived from observed output. An expectation is read out of the
implementing service first, which is what makes a disagreement between
row and code evidence of a defect rather than a reason to edit the row.

Rows under canaries/ are built to fail, one per assertion mechanism. A
canary that passes means that mechanism has stopped asserting anything, so
it voids the whole run instead of counting as success.

The ledger records what is known broken and since when. The daily job
reports only transitions: a failure already known stays silent, a fix is
announced once, and the entry is deleted on its second consecutive pass,
so the file holds nothing that is no longer true.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
## Context

The previous attempt at a billing suite produced 75 findings, 36 of them
affecting money, before it was abandoned. Most of its 1,163 rows were
coverage rather than signal, but a handful of pieces were expensive to
learn and worth keeping.

## Description

Lifted verbatim from golden-billing-harness at 8cd803568, kept in a
directory outside spec/ so nothing autoloads a half-ported file. The
README records where each piece came from and whether it is to be ported,
mined for scenarios, or read once and dropped.

Files are deleted as they are absorbed. An empty salvage directory is the
signal that the port is finished.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
## Context

An independent audit pointed the suite at an unmigrated database. Every
row errored, so no canary passed, so the canary-broken count stayed at
zero and the run exited 0 — a green build in which nothing had been
asserted at all. That is the same shape of failure the canaries exist to
catch, one level up: the guard was watching for canaries that pass, not
for canaries that never reached an assertion.

## Description

A canary is proven only by failing. Any other outcome, including erroring
on the way, means the mechanism it guards went unexercised, so the run is
void and exits 2. The ledger refuses to move on such a run, and also
refuses one containing no canaries at all, which is what a hand-filtered
debugging run looks like.

The report now distinguishes canaries that passed from canaries that never
got as far as asserting, because the two call for different fixes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
## Context

An independent audit found several places where a row or a canary could
report success without having asserted anything, plus two workflow
conditions that reacted to the wrong exit codes.

## Description

A plan override is now observable. Previously the row proving an override
had expectations identical to its control, so an API that accepted and
then ignored plan_overrides would have satisfied both — and the ledger
would have announced the override defect fixed and deleted it. Rows can
now assert whether the subscription's plan is a child plan, asserted true
on the overridden row and false on its control, which is what makes the
axis provably drive a difference rather than merely being labelled.

The plan's id cannot be asserted directly, being a fresh uuid each run;
the parent link carries the same fact deterministically.

Two canaries covered the same comparison path, so one is gone. Two paths
had no canary at all and now do: an entity the row expects but the
scenario never produces, and the comparison of a fee's fields once it has
been matched by identity. The second was verified the only way a canary
can be — by removing the line it guards and watching it pass.

The daily job treats a harness that never ran as the loudest failure
rather than the quietest, and no longer builds a report from a results
file that a failed run may have left stale.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
## Context

Billing coverage needs a reliable on-demand run before scheduled execution is enabled.

## Description

Remove the schedule and default to report-only runs. Retain results and logs, reject incomplete execution, and persist ledger cleanup without requiring a new alert. Fix Ruby lint violations and add licence controls and paired scenarios for invoice settings, draft repricing, wallets, and preview differences.
## Context

Row validation must remain available without booting Rails.

## Description

Keep the timeline forwarding methods independent of ActiveSupport and document the targeted lint exception.
## Context

The unused previous harness accounts for most of the billing matrix PR and makes the active suite harder to review.

## Description

Remove the historical runner, schema, rows, and state from the current tree and link to their preserved Git revision. Retain the compact feature-combination reference used by the maintainer and update documentation to the active helper interfaces.
@annvelents
annvelents marked this pull request as ready for review September 10, 2026 13:15
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

Automated pre-review (advisory, not a required check) — verdict: HOLD · CI green

HOLD — errored runs lose their report, and the new harness has no automated test coverage.

  • .github/workflows/billing-matrix-daily.yml:122: the .errored == 0 check fails before the ledger, Slack, and Actions-summary steps; GitHub Actions then skips those steps via the implicit success() condition. Move the failure gate after reporting or make the reporting path run for exit-code-0 results containing errored rows.
  • Add focused specs for the runner and ledger. The five canaries cover only selected end-to-end comparison paths; they do not cover row validation/selection, canary result accounting, fee ambiguity, or ledger transitions, and there are no specs for this new code.

## Context

An errored scenario failed the workflow before its report was visible, and the harness lacked focused regression specs.

## Description

Report incomplete runs before failing the job while preventing ledger publication. Cover row validation, selection, fee comparison, canary accounting, and ledger transitions with unit specs. Load the scenario context only when executing the CLI so importing it for tests cannot clean the database.
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

Automated pre-review (advisory, not a required check) — verdict: HOLD · CI green

HOLD — the new harness has false-success and stale-ledger paths, plus one advertised setup combination cannot be built.

  • BillingMatrix::CLI#call does not return after abort_harness("no rows matched"); an unmatched --id/--area therefore writes an empty result and ultimately exits 0. Return the harness-failure code and add a regression spec.
  • The workflow computes ledger.yml from the checked-out base before resetting billing-matrix/ledger, so a rerun while its PR is open can re-report/reset a pending failure or leave a stale failure when the row passes. Use the current ledger branch as the input state and cover the rerun transition.
  • World permits billing_entity.tax_codes but updates the billing entity before creating setup.taxes, so the existing ManageTaxesService cannot resolve those codes. Create taxes first and cover this supported setup combination.

## Context

Manual runs could report success without matching rows, reuse stale ledger state, or fail to apply taxes declared in the same setup.

## Description

Reject empty selections before booting the database, use pending ledger PR state across reruns, and create taxes before assigning them. Add regression coverage for these cases and document ledger behavior before merge.
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

Automated pre-review (advisory, not a required check) — verdict: HOLD · CI green

HOLD — shard isolation is checked only after destructive database cleanup.

  • billing_matrix/run.rb:72-75 calls boot! before assert_shard_isolation!, while runner/boot.rb:35 deletes every row. A shard pointed at a shared _test database wipes it before being refused. Validate the shard/database pairing before any cleanup and add a regression spec proving cleanup is never reached on a mismatch.
  • Make the advertised --rows FILE form work and defer control validation until all repeated paths are combined: Row.load_all always appends /**/*.yml and validates each path in isolation, so file inputs and cross-path controls fail.
  • Propagate row pins into results/ledger or remove the contract claim that the ledger uses them; Results#row_to_h drops the field and Ledger never reads it.

## Context

Shard isolation ran after destructive cleanup, repeated row inputs were validated separately, and reports lost finding IDs.

## Description

Check shard database isolation before cleanup. Load file and directory inputs as one corpus before validating references. Carry finding IDs through results and ledger reports while preserving transition deduplication. Add regression coverage and update the runner contract.
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

Automated pre-review (advisory, not a required check) — verdict: PASS · CI green

PASS — No blocking correctness or consistency issues found. The harness, canary safeguards, ledger transitions, publication gates, and focused specs align with the existing API helpers and serializers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant