ci(b9): assert embed_provider/embed_model with a negative test, read back the */30 round, correct B9-4's rationale - #80
Open
tornidomaroc-web wants to merge 3 commits into
Open
Conversation
The monitor asserted ok, embed_dim and supabase_configured, and NEVER checked which provider or model produced the vectors. Register #56 leaves `FROM python:3.11-slim` a mutable tag with no transitive pins and register #55 rebuilds the image on every push to `main`, so a dependency resolution that changed the embedding MODEL while holding the dimension at 1024 would write vectors from a different model into the same vector(1024) column, pass all three existing assertions, and silently degrade retrieval against every chunk already stored. N11 compared these two fields by hand against the pre-merge capture. That was a human diffing two strings in a chat window: not a standing check, and it did not survive the session. Two `jq -e` lines make it standing. The literals are grounded in three independent /health reads on three successive deployments -- 6e03e8bd, 35cbf150 and 6e740a39 -- all byte-identical: {"ok":true,"embed_provider":"voyage","embed_model":"voyage-3-large", "embed_dim":1024,"supabase_configured":true} Checked before writing, because this workflow's own sibling shipped the defect: the existing assertions do NOT pass vacuously the way PR #71's `docker run` without `-i` did. `jq -e` yields 0 only when the last output was neither false nor null; a false comparison is 1, a missing field compares as null and is 1, malformed JSON or a missing file is 2, an absent binary is 127 -- every one of those trips the explicit `|| { exit 1; }`. The redirect is `> /dev/null`, a redirect and not a pipe, so unlike the `| tee` case in ingestion-image.yml the exit status is not replaced. The monitor has been green on something real. B9-4 said this should not wait for B9, and it is the cheapest standing guard against the one hazard N10's digest pin is otherwise the only defence for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… negative test (B9-3) **Zero behaviour changes. The diff is comments only** -- verified mechanically: no changed line outside a `#` comment, the cron line is byte-identical, and all five `jq -e` assertions and both probes are untouched. What changes is that the comment above the cron line stops promising a result and starts carrying one. ## B9-3 read back: the criteria are void, and that is the first finding 248 scheduled runs over 329.8 hours, 247 intervals, `*/30` unchanged since the PR #77 merge at 2026-08-03T13:17Z. All figures are `event: schedule` runs only; the four `workflow_dispatch` runs (2026-08-02T14:59Z, 2026-08-12T11:54Z and the two negative-test dispatches below) are excluded from every interval computation. **PR #77's NO CHANGE band could not have been reached by any outcome.** It required a full-cycle mean of 95.7-129.5 min *while* the daytime mean sat at 57.7-78.1 -- a full/daytime ratio of 1.23-2.24. The measured ratio is **1.10** over the whole `*/30` record, **1.17** counting only the runs that actually probed, and 1.10-1.17 in every sub-window on either series. The band was only satisfiable if overnight delivery stayed as bad as it was on the one night that produced the baseline, whose three overnight intervals (210.8, 218.7, 225.7) forced a 1.66 ratio out of a ten-interval sample. **Ten intervals were not a baseline; they were one bad night, and the criteria promoted that night's shape into a structural requirement.** Every candidate read window -- 24h, 48h, 72h, 96h, 120h, 168h and the full record -- lands in **no bucket at all**. The methodology reproduces the baseline exactly before being applied forward: the documented 16:08:47-22:56:08Z window recomputes to 7 runs, 6 intervals, **67.9 min**, and the full pre-change record to 10 intervals, **112.6 min**, with overnight intervals of 225.7 / 218.7 / 210.8. Same code, same answers. **This is recorded as a defect in the criteria rather than re-fitted into a bucket.** Choosing whichever band the data happened to sit nearest would have produced a "result" that no future reader could distinguish from a real one. ## The second finding survives the first, and is the answer the round was for **The requested rate is not the binding variable.** With the cron line unchanged, delivery ran **146.1 min** mean over Aug 3-6, **79.4 min** over Aug 7-14, and **48.2 min** over the last 48 hours -- from 10-23% of the requested 48 runs/day in the first window to 63-65% on Aug 15-16. A threefold swing with no input moved dwarfs the 112.6 -> 80.1 difference the round was built to detect, so **no comparison of `*/15` against `*/30` can be read out of this record.** Repo activity does not explain it: five commits landed across the entire window and none on Aug 14-16, spanning the step change. Delivery is set inside GitHub and is **not stationary**. The question is retired. A third cron value would measure the drift, not the value. **Not reverted to `*/15`**, though Aug 3-6 alone read 146.1 and tripped PR #77's WORSE threshold. Reverting on a window that later regressed to 48.2 would be reacting to a regime rather than to the setting, and would forfeit the only constant-input series there is. ## Third finding: the concurrency block's own stated hazard came true **The worst blind window is 596 minutes -- 9.9 hours, 2026-08-06T14:29Z to 2026-08-07T00:25Z -- and it is not simply GitHub being slow.** Two scheduled runs inside it, **31119684416** (16:23:58Z) and **31125404749** (18:12:55Z), were **cancelled before they started**: job `probe` cancelled, zero steps, no probe performed, no assertion evaluated. These are the only two non-successful scheduled runs in the entire `*/30` record. `cancel-in-progress: false` does **not** stop GitHub cancelling an already-**queued** run when a newer one enters the same concurrency group. On a day when delivery bunched, that is what happened, twice. The comment on that very block says a cancelled probe *"reports nothing, and reported nothing is indistinguishable from was never scheduled"*. **That hazard is not hypothetical. It came true here, and it produced the worst gap on record.** **Counting scheduled runs regardless of outcome hides it entirely** -- that series reads 372.6 min for the same window, a 60% understatement. Every figure above is reported on both series for that reason; the void-criteria conclusion holds on both. 596 min is the honest worst case and it, not any mean, is the figure to use when asking how long drift can sit unseen. It still clears the nine-day bar by a factor of 22. **Not fixed here.** Removing the concurrency group, or scoping it so a queued probe is never displaced, is a behaviour change and does not belong in a comment-only commit. It is the strongest argument yet for the dead-man's-switch already recorded as an accepted risk further down this file, since two silent cancellations are exactly the "monitor stops and says nothing" shape. ## The embed_provider / embed_model assertions were negative-tested The previous commit on this branch (d82905d) argued from `jq`'s documented exit codes that a false comparison would red. **That is a man page, not a run.** Both assertions have now been proven to fail the job against live production, on a scratch branch cut from d82905d, flipping one literal per dispatch and nothing else. The scratch branch has been deleted; **these run logs are deleted by GitHub's 90-day retention on or about 2026-11-15, which is why the evidence is transcribed here rather than cited.** Two dispatches, not one, and the order is load-bearing. The four assertions are sequential statements in one `run:` block and each ends `|| { echo ...; exit 1; }`, so a red terminates the step. Flipping `embed_provider` alone would red at that line and **`embed_model` would never execute** -- zero evidence about the assertion B9-4 exists for. Flipping `embed_model` alone proves its failure path *and*, by the fact that control flow reached it, proves `embed_provider`'s line runs and passes against live production. Neither single dispatch renders `embed_provider`'s own error, which had never been emitted once. **Run 32019016890** -- `.embed_model` literal flipped to `NEGATIVE-TEST-B9-4-NOT-A-REAL-MODEL`. Job FAILED, exit 1, `Probe /health` red, `Probe unauthenticated POST /convert` skipped, no `HEALTH OK`. Emitted verbatim (the workflow writes `::error::`; the runner log renders it `##[error]`): ##[error]embed_model is not "voyage-3-large" (got "voyage-3-large"). embed_dim can still be 1024 and every other assertion here can still pass while this is wrong -- that is exactly the gap this line closes. New chunks are being embedded by a DIFFERENT model than the stored ones. Do not relax this assertion. **Run 32019111049** -- `.embed_model` restored, `.embed_provider` flipped to `NEGATIVE-TEST-B9-4-NOT-A-REAL-PROVIDER`. Job FAILED, exit 1, same shape: ##[error]embed_provider is not "voyage" (got "voyage"). The embedding provider changed underneath a vector(1024) column that already holds voyage-3-large vectors. Retrieval is degrading silently against every stored chunk. Do not relax this assertion. Both runs read production healthy throughout -- `HTTP 200` and, byte-identical in both, `{"ok":true,"embed_provider":"voyage","embed_model":"voyage-3-large","embed_dim":1024,"supabase_configured":true}`. The `(got ...)` substitution therefore echoes the *correct* value back, which is what makes each line self-evidently synthetic: a genuine drift red must print a value that differs from the one it names, and neither of these does. The `\"voyage\"` escaping in the provider message is confirmed to render as `"voyage"`, which had never been observed before this run. ## What the negative test does NOT prove **The literal was flipped, not `/health`'s output.** What is established is that these assertions execute in order, that a false comparison exits non-zero on this runner with this `jq`, that the `|| { ...; exit 1; }` block fires, that the annotation renders, and that the step's failure fails the job. What is *not* established is that Railway can produce the disagreement -- no deployment served a wrong model during this test, and none was made to. The proxy is sound because `==` is symmetric and a missing field compares as `null` and also reds, but it is a proxy, and a reader should not take these two runs as evidence that a real model swap has ever been observed or caught. It has not. ## Scope `.github/workflows/production-monitor.yml`, comments only. **ZERO files under `src/`. ZERO under `services/`. ZERO under `docs/`.** No probe logic, no assertion, no retry count, no `main.py`-derived `/convert` expectation moves. B9-3's row in `docs/b9-queue.md` stays **open** -- it is closed by the docs commit that carries these figures into the four B9-2 sites, which is deliberately not this commit. No registers open or close here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…4's false rationale (registers #59, #60; #58 reserved unwritten) The record for this PR. Seven items, all consequences of the two commits below it. **ZERO files under `src/`. ZERO under `services/`. No behaviour change anywhere in this PR** -- the only non-comment lines it ships are the two assertions `d82905d` added. ## Scope widened past docs-only, deliberately B9-4's false rationale sits in three places: `d82905d`'s commit message, the workflow's comment block, and `b9-queue.md`. **Correcting the docs and leaving the workflow would have manufactured a contradiction between two records inside a single PR** -- `b9-queue.md` saying the dependency mechanism does not exist while the file it describes still says it does. A correction that creates a contradiction is not a correction. So this commit includes a **comment-only** change to `.github/workflows/production-monitor.yml`, verified mechanically to touch no line outside a `#`. Docs-only purity is a preference; non-contradiction is the requirement, and the two collided. **`d82905d`'s message is immutable and stays wrong.** Both corrected sites point at it by SHA so a reader who finds the dependency-resolution argument there lands on the correction. That is the best available outcome, not a good one. ## Ruled: what the record carries about a wrong rationale B9-4 argued that register #56's mutable `FROM python:3.11-slim` tag plus register #55's rebuild-on-push let *a dependency resolution* change the embedding model. **It cannot.** The model is `os.environ.get("VOYAGE_MODEL", "voyage-3-large")` (`services/ingestion/main.py:19`), sent explicitly as `"model": VOYAGE_MODEL` in a raw `httpx` POST (`:141`). No SDK default exists to shift. `pip` cannot move that string and neither can a base-image rebuild. **The guard is still right and stays.** It catches a `VOYAGE_MODEL` or `EMBEDDING_PROVIDER` change on Railway, or an edit to `main.py:19`/`:43` shipping without the workflow literals moving. Both are real drift. **The record carries that a wrong rationale which produced a correct guard is still a record defect**, and says why: the hazard was reasoned about at the wrong layer, and nobody read the twenty lines of `main.py` that would have settled it in a minute. Recording only "the guard was justified after all" would preserve the outcome and discard the lesson. ## Ruled: the void criteria earn a rule, and it goes in the file that hosts rounds Voiding B9-3's criteria retires that round and nothing else. Three checkable rules now sit in `b9-queue.md`'s header, where the next person opening an empirical round will hit them: every bucket reachable and the set exhaustive; no two coupled statistics required simultaneously without a stated tie-break; a band inherits the authority of the sample it came from. **Short and checkable on purpose** -- a docs commit closing one item may leave a rule behind, but it has no business leaving a methodology behind. Each rule names the concrete way B9-3's criteria broke it, so none of it is abstract. ## Register #58 does not exist and was NOT reconstructed The owner identified a register #58 as belonging to this beat. **It is not in this repository.** §4 runs 1-57; the highest is #57 (account identity, 2026-08-10); no `#58` appears anywhere in `docs/` -- every apparent hit is *pull request* #58. Its content could not be recovered from the record and **was deliberately not inferred**: an invented register is indistinguishable to a later reader from a ruled one, which is the exact defect §4 exists to prevent. A placeholder row holds the number, because #58 being claimed-but-absent is not only a gap in the record, it is a **numbering hazard** -- without the placeholder, the two registers this commit opens would have taken #58 and #59 and collided with whatever the owner supplies. They are numbered **#59** and **#60** instead. The row asserts nothing and says it asserts nothing. ## The seven items 1. **B9-3 CLOSED.** Criteria void -- the NO CHANGE band required a full/daytime ratio of 1.23-2.24; measured is 1.10 (all scheduled runs) and 1.17 (runs that actually probed), so every candidate window landed in no bucket. Recorded as a defect, not re-fitted. Methodology validated by reproducing the documented baseline first (7 runs / 6 intervals / 67.9 min; full record 112.6 min). 2. **The four B9-2 sites re-corrected** -- §9.1, §10's preamble, the V1 and N9 evidence cells. They were corrected on 2026-08-10 to `~68 / ~113 min`, which were the **baseline** means, not the `*/30` result. All four now carry **worst observed blind window 9.9 h (2026-08-06), typical 50-80 min, non-stationary.** §9.1's sentence says "at most", so a mean was always the wrong statistic there. 3. **§9.1 corrected** as part of item 2, and it additionally now names register #60, since two of the 9.9 hours' constituent runs never probed. 4. **Register #58 reserved, unwritten** -- above. 5. **Negative-test citations**: runs `32019016890` and `32019111049`, the conclusion, the two-dispatch reason, and the proxy limit. The verbatim `::error::` text is **not** duplicated here -- it is in this branch's second commit message, and docs carry the index while git carries the evidence. 6. **B9-4 CLOSED, rationale corrected, register #59 opened** -- `/health` reports a self-declared string, so a server-side repoint of `voyage-3-large` is undetectable and a per-row model column on `chunks` would not close it either. **The provenance idea raised this session is ruled DOWN, behind #47**, because `chunks.created_at` plus retained `chunks.content` (`20260501_rag_pgvector.sql:16,19`) already bound recovery to a re-embed over the last-green-probe window. Closure is an embedding canary; its own PR. 7. **Register #60 opened** -- runs `31119684416` and `31125404749` were cancelled before starting on 2026-08-06, zero steps, no probe. They produced the 9.93 h worst case and are the concurrency block's own stated hazard coming true. `cancel-in-progress: false` does not protect an already-queued run. Counting scheduled runs regardless of outcome hides it -- that series reads 372.6 min for the same window, which is how the figure was first written **and corrected inside this same PR**. Its own PR. ## Not here The embedding canary, any `chunks` migration, and the concurrency fix. All three are behaviour changes and each earns its own PR. `PROGRESS.md` §7's 2026-08-10 block is superseded by a new block, never edited. Phase 7 stays unstarted; `PIVOT_PLAN.md` untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
Three commits. The only non-comment line this PR ships is the pair of
jq -eassertions in commit 1 — verified mechanically across the whole range. ZERO files undersrc/. ZERO underservices/. No behaviour change.d82905dembed_providerandembed_modelin the monitor (closes B9-4)ab11810*/30round, void its criteria, record the negative test (B9-3)73439b8The guard is negative-tested, not argued from a man page
d82905dreasoned fromjq's documented exit codes that a false comparison would red. That is a man page, not a run. Both assertions are now proven to fail the job against live production, on a scratch branch cut fromd82905d, one literal flipped per dispatch, branch deleted afterwards:32019016890—embed_modelflipped → job FAILED,Probe /healthred.32019111049—embed_providerflipped → job FAILED, same shape.Two dispatches were required, not one. The assertions are sequential statements in one
run:block and a red exits the step, so flippingembed_provideralone would have leftembed_modelunexecuted — zero evidence about the assertion B9-4 exists for. Verbatim::error::output for both is transcribed intoab11810's message, because GitHub deletes those logs on or about 2026-11-15 and git is the only store that outlives it.What it does not prove: the literal was flipped, not
/health's output. The assertions execute and fail the job; Railway was never made to serve a wrong model.Three findings that changed what this PR says
1. PR #77's read-back criteria were void. The NO CHANGE band required a full/daytime ratio of 1.23–2.24; measured is 1.10 across all scheduled runs and 1.17 across runs that actually probed. Every candidate window — 24h through the full 329.8-hour record — landed in no bucket. Recorded as a defect in how criteria are written, not re-fitted into whichever band the data sat nearest. Methodology was validated by reproducing the documented baseline first (7 runs / 6 intervals / 67.9 min; full record 112.6 min).
2. The requested cadence is not the binding variable. With the cron line unchanged, delivery ran 146.1 min (Aug 3–6), 79.4 (Aug 7–14), 48.2 (final 48h). A threefold swing with no input moved dwarfs the difference the round was built to detect. Question retired; not reverted to
*/15.3. The worst blind window is 9.93 h, and it is partly self-inflicted. Runs
31119684416and31125404749were cancelled before starting on 2026-08-06 — zero steps, no probe.cancel-in-progress: falsedoes not protect an already-queued run, and the concurrency block's own comment says a cancelled probe "reports nothing, and reported nothing is indistinguishable from was never scheduled." Counting scheduled runs regardless of outcome reads 372.6 min for the same window — a 60% understatement, which is how the figure was first written in this PR and then corrected inside it.B9-4's rationale was false. The guard is still right.
B9-4 claimed register #56's mutable base tag let a dependency resolution change the embedding model. It cannot: the model is
os.environ.get("VOYAGE_MODEL", "voyage-3-large")(services/ingestion/main.py:19) sent explicitly in a rawhttpxPOST (:141).pipcannot move that string. The guard catches Railway env-var drift andmain.pyedits shipping without the literals moving — both real. The false claim is corrected inb9-queue.mdand in the workflow comment;d82905d's message is immutable and stays wrong, and both corrected sites point at it by SHA.This is why the docs commit is not docs-only: correcting one record and not the other would have manufactured a contradiction between two files in the same PR.
Registers
voyage-3-largeserver-side, nothing reds. A per-row model column onchunkswould not close it either. The provenance idea raised this session is ruled DOWN, behind docs: register #39 — repo↔live schema parity invariant; cross-ref #23; advance headers #47 —chunks.created_atplus retainedchunks.contentalready bound recovery to a re-embed over the last-green-probe window. Closure is an embedding canary.#58hit is pull request refactor: hoist KnowledgeBase.language domain to a single source of truth #58). It was deliberately not reconstructed by inference, and a placeholder holds the number so docs: register #42 KnowledgeBase.language app-level closure + complete remaining surface #59/refactor: import hoisted Tier in rate-limit instead of inlining the union #60 do not collide with it. This PR should not merge until refactor: hoist KnowledgeBase.language domain to a single source of truth #58 is supplied or the row is confirmed as a deliberate placeholder.Not here
The embedding canary, any
chunksmigration, and the concurrency fix — all three are behaviour changes and each earns its own PR.🤖 Generated with Claude Code