Skip to content

feat(online): production failures become regression tests, safely (P6) - #54

Merged
lopadova merged 1 commit into
feat/p5-finopsfrom
feat/p6-promote-online
Aug 24, 2026
Merged

lopadova merged 1 commit into
feat/p5-finopsfrom
feat/p6-promote-online

Conversation

@lopadova

Copy link
Copy Markdown
Contributor

Sixth item of the roadmap. Stacked on #53 (P5) → #52 (P4) → #51 (P3a) → #50 (P2).

The thing this changes

A golden dataset written at a desk encodes what its authors already thought to worry about. The questions that actually break a system are the other ones — the phrasing no designer would choose, the edge case that only exists in real inventory — and they arrive at 11pm from a real user.

Online monitoring was already scoring exactly those interactions. Nothing turned them into tests.

php artisan eval-harness:promote-online rag.factuality \
    --merge=database/evals/rag.factuality.yaml --since=7 --limit=25
12 promoted, 84 already present, 3 duplicates.

Tonight's incident becomes tomorrow's red build instead of an anecdote in a retro.

First: the interaction has to be kept, and that is the risky half

eval_harness_online_scores stored a score, not the text. Additive migration adds nullable input / expected_output / actual_output / redactor / redacted_at, filled only when retention is switched on — because keeping production text is a different decision, with a different legal basis, from keeping a number.

The row you most want is, for exactly that reason, the row most likely to contain a name, an order number or a medical detail. So:

  • Online\Redactor — one method, text in, text out, so a host that already owns a text redactor plugs straight in. The package walks nested input at any depth and applies it to string keys too (a map keyed by email address is not hypothetical).
  • This package ships no redactor. A redactor trustworthy with production text is a package of its own — laravel-pii-redactor, your own regex, a classifier — and which is right depends on jurisdiction, data and retention policy. An eval harness with its own half-good PII regex is a package quietly promising a compliance property it cannot keep.

Three defaults, each chosen against convenience

Default Why
retention off somebody has to turn it on deliberately
require_redactor on → retention with nothing bound raises the failure that actually happens is "enabled in a hurry, redactor next sprint, six months of customer questions in a table nobody remembers agreeing to"
a throwing redactor drops the interaction losing a row is an inconvenience; storing the one string the redactor choked on is an incident — and the exception message deliberately does not carry that string, or the log becomes the leak

Redaction happens at the boundary, in the job — never written raw and cleaned up later, because "later" is after the backup, the read replica and the log shipper each took a copy. redactor and redacted_at are stored per row, because "which redactor handled this?" is asked months later and a boolean cannot answer it.

Duplicates solve themselves

A nightly promotion promotes the same recurring failure every night, so dedup is not a nicety. Rows dedupe by RowHash — the same content hash the P2 regression gate joins on. Consequences worth naming:

  • a row already covered, including one somebody wrote by hand, is not promoted again;
  • a promoted row keeps its history from the moment it lands, because it joins to past and future runs on the hash the comparator already uses;
  • the row id derives from the hash (online-9f2c4a1b3e7d), not the database id, so promoting from a second environment or re-promoting after a truncation does not renumber a dataset somebody has already committed.

Failure modes handled

  • Unredacted rows are skipped unless --allow-unredacted — promoting raw production text copies it into a committed file, and a repository is forever — and the skip is announced, or somebody expects a row and concludes it never existed. Legacy scores with no retained text get their own line, so an upgraded install does not silently produce a dataset of empty questions.
  • A corrupt --merge target stops the command. The promoter reads an unreadable target as "no existing rows", which is right for a missing file and catastrophic for a corrupt one: writing back would replace a curated dataset with this run's handful of promotions. Same fallback, opposite consequences.
  • --limit keeps the worst-scoring rows, not whatever was inserted first.
  • Failures only by default: a dataset of rows the pipeline already gets right can only ever stay green.

Composes with the rest of the roadmap

Every promoted row is tagged promoted-from-production, which puts them in one cohort — so a P4 briefing can say "7 of 9 failures are tagged promoted-from-production". That is a very specific diagnosis: the pipeline is failing on real traffic and passing the tests somebody imagined.

Verification

  • 25 new tests; suite 1111 → 1136 (3078 assertions).
  • composer validate --strict, phpunit, PHPStan level 6, Pint — all green locally.
  • Docs: docs-site/docs/guides/promoting-production-failures.md + nav; README feature bullet; config block with the reasoning inline; docs/PROGRESS.md + docs/LESSON.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EZ4u82zdoa8naYm75kppMC


Generated by Claude Code

A golden dataset written at a desk encodes what its authors already thought to
worry about. The questions that actually break a system arrive at 11pm from a
real user, and online monitoring was already scoring them -- nothing turned them
into tests.

- The interaction has to be kept first: eval_harness_online_scores stored a
  score, not the text. Additive migration adds nullable input/expected_output/
  actual_output/redactor/redacted_at, filled only when retention is enabled.
  Keeping production text is a different decision, with a different legal basis,
  from keeping a number -- so it is a different switch, and it is off.
- Online\Redactor: one method, text in, text out. This package ships none. A
  redactor trustworthy with production text is a package of its own, and an eval
  harness with its own half-good PII regex is a package quietly promising a
  compliance property it cannot keep.
- Online\InteractionRetention: require_redactor defaults on, so retention with
  nothing bound raises rather than storing raw text -- the failure that actually
  happens is "enabled in a hurry, redactor next sprint, six months of customer
  questions in a table". A throwing redactor drops the interaction, and the
  exception deliberately does not carry the text it failed on. Nested input is
  walked at any depth, string keys included.
- Redaction happens at the boundary, in the job, never written raw and cleaned
  up later: later is after the backup, the replica and the log shipper each took
  a copy.
- eval-harness:promote-online: failures by default, --all/--max-score/--since/
  --limit (which keeps the worst-scoring rows), --merge writing back in place so
  the diff is pure additions.
- Dedup by RowHash, the same content hash the regression gate joins on: a
  nightly promotion is idempotent, a hand-written row covering the case is not
  duplicated, and a promoted row keeps its history from the moment it lands. Row
  ids derive from that hash, not the database id, so promoting from a second
  environment does not renumber a committed dataset.
- Unredacted rows are skipped unless --allow-unredacted, and the skip is
  announced. A corrupt --merge target stops the command rather than replacing a
  curated dataset with this run's promotions.

Tests 1111 -> 1136. composer validate --strict, phpunit, PHPStan level 6 and
Pint all green.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01EZ4u82zdoa8naYm75kppMC
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying eval-harness with  Cloudflare Pages  Cloudflare Pages

Latest commit: a95d492
Status: ✅  Deploy successful!
Preview URL: https://e9ab89b1.eval-harness.pages.dev
Branch Preview URL: https://feat-p6-promote-online.eval-harness.pages.dev

View logs

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying laravel-patent-box-tracker with  Cloudflare Pages  Cloudflare Pages

Latest commit: a95d492
Status: ✅  Deploy successful!
Preview URL: https://c519f58c.laravel-patent-box-tracker.pages.dev
Branch Preview URL: https://feat-p6-promote-online.laravel-patent-box-tracker.pages.dev

View logs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a95d4927dc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +131 to +133
foreach ($input as $key => $value) {
$redacted[$key] = $this->redactValue($redactor, $value);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Redact top-level input keys before retention

When the production input itself is keyed by sensitive data (for example ['[email protected]' => ...]), redactInput() passes the top-level key through unchanged even though nested keys are redacted. Enabling retention with a correctly bound redactor can therefore still persist raw PII in the input JSON column; apply the redactor to string keys at this outer level as well.

Useful? React with 👍 / 👎.

Comment on lines +218 to +224
$document = ['name' => $name];

if ($metrics !== []) {
$document['metrics'] = $metrics;
}

$document['samples'] = $samples;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve dataset options when merging

When --merge targets a dataset with repetitions: N, this reconstruction emits only name, metrics, and samples, silently deleting repetitions (as well as explicit schema metadata) from the file. The next load consequently defaults to one repetition, changing the experiment's execution count and statistical resolution merely because rows were promoted; preserve the existing document-level dataset fields during an append.

Useful? React with 👍 / 👎.

Comment on lines +135 to +138
// Worst first, then newest: when a limit truncates the batch, the rows
// that survive should be the ones the pipeline handled worst.
/** @var list<OnlineScore> $rows */
$rows = $query->orderBy('score')->orderByDesc('judged_at')->limit($criteria->limit)->get()->all();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Apply the promotion limit after eligibility checks

When the worst-scoring rows are already in the merge target, unretained, or disallowed as unredacted, this query truncates to --limit before those rows are skipped. A nightly run can therefore repeatedly inspect the same existing worst 50 rows, report nothing new, and never reach later eligible failures; enforce the limit on successfully promotable rows rather than on the raw query result.

Useful? React with 👍 / 👎.

@lopadova
lopadova merged commit 4f928c4 into feat/p5-finops Aug 24, 2026
2 checks passed
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.

2 participants