chore(drift-sync): needs-human model-family decision (2026-08-04) - #350
Closed
copilotkit-devops-bot[bot] wants to merge 1 commit into
Closed
chore(drift-sync): needs-human model-family decision (2026-08-04)#350copilotkit-devops-bot[bot] wants to merge 1 commit into
copilotkit-devops-bot[bot] wants to merge 1 commit into
Conversation
commit: |
jpr5
added a commit
that referenced
this pull request
Aug 4, 2026
…amilies (wrong modality) (#343) <!-- drift-changeset: eaa8db65f5647493 --> <!-- drift-proposal-note: drift-proposals/openai-gpt-live-transcribe-new-family.md --> <!-- drift-proposal-note: drift-proposals/openai-gpt-transcribe-new-family.md --> > **⚠️ Do not remove the HTML comment markers above.** `fix-drift.yml` dedups by grepping open PR > bodies for `drift-changeset: <key>` and `drift-proposal-note: <path>`. Removing them makes the next > scheduled run treat this changeset as unproposed and open a duplicate needs-human PR — which is > exactly what happened when this body was rewritten (see #350). Classifies the two OpenAI model families that shipped 2026-07-28 — `gpt-transcribe` and `gpt-live-transcribe` — and pins the registry surfaces that classification touches. ## The decision Both families are **transcription-only**. `gpt-transcribe` serves `/v1/audio/transcriptions` and realtime transcription sessions; `gpt-live-transcribe` is realtime-only. **Neither answers on `/v1/chat/completions`.** - **`excludeFamilies.openai`** (`src/__tests__/drift/model-registry.ts`) — the text-listing canary. Excluded, because they are the wrong modality. This mirrors the existing entries for `gpt-4o-transcribe`, `whisper-1` and `gpt-realtime*`, and follows established prior art for keep-but-not-live families (`gemini-pro`, `gpt-3.5`). - **`knownVoiceModelFamilies`** (`src/__tests__/drift/voice-models.ts`) — the voice/realtime canary, a deliberately disjoint registry. Added, because that is the remediation the collector itself prescribes: *"Unknown realtime model detected — add to `knownVoiceModelFamilies`"*. These are two different registries with two different jobs. Excluding from the text canary is not the same as silencing detection — the voice canary is where these families are tracked, and membership there is the tool's own sanctioned resolution of the finding it raised. The `Decision:` lines in the two `drift-proposals/` notes are deliberately **not** set to `include`. The drift-sync auto-apply path writes exclusively into `includeFamilies`, so following the note's own boilerplate instruction would misclassify both families as chat-mocked. The decision is recorded as prose; wiring an `exclude` decision value is tracked separately. ## Live evidence `pnpm test:drift` against the real provider APIs on this branch: **136 passed, 0 failed.** Both canaries that are **red on `main`** are **green on this branch**: - `✓ live /models contains no unclassified family` - `✓ canary: GA realtime models available` So this branch alone resolves the drift. `drift-live-pr` passes in CI. Note that the `drift` job itself is gated `if: github.event_name != 'pull_request'` and therefore **skips on PRs** — a live `Drift Tests` dry-run on the final sha is the pre-merge gate, confirming the two families no longer flag, the realtime canary is in the expected state, and nothing else started flagging. ## What is in this PR, stated plainly 8 commits, **9 paths**. It is not a small diff: roughly **200 lines are pin machinery** (`logic-pin.test.ts` is `+193/-1`), with the classification itself being a handful of lines. Included deliberately rather than deferred, because two of these pins were **mandatory findings from review**: - `excludeFamilies.openai` `DATA_FROZEN` re-pin — required by the membership change. - Pins for `knownVoiceModelFamilies`, `gaRealtimeModels` and `isVoiceModelId`, plus the two functions that consume the seed sets. Before this, **adding a family to the voice registry tripped no guard at all**, and neutering `detectVoiceModelDrift` to `hasGA = true` — which permanently disables the "provider removed a GA realtime family" alarm — passed the entire 4,788-test suite with nothing watching it. - An anchored assertion so the live text-lane canary cannot be silenced by neutering its wrapper. All pins are `^`-anchored and use behavioural anchors alongside checksums: a checksum reports "something changed", a behavioural anchor reports "this no longer detects an unclassified family". Every one was proven by mutation — the guarded value was broken, the guard observed failing, the mutation reverted, green confirmed. A pin nobody has watched fail is not a guard. Also: un-mangles the `src/__tests__/` paths cited in the two notes (Prettier had eaten the underscores, so the committed audit trail cited files that do not exist), adds `drift-proposals/` to `.prettierignore` so that cannot recur, and drops two `gemini-1.5` notes whose decision was already applied in `a3dc250`. **This PR touches zero files under `scripts/`.** That is load-bearing: it means the change cannot alter delta keys, so no base/head skew and no transitional gate failure is possible. ## Split out of this PR This branch previously carried 19 commits. The collector, delta-keying and test-tooling work has been moved out, because three review rounds each found real mandatory defects and almost all of them were in guards the review loop itself had added — a scope signal, not a quality signal. - **#349** (draft) — drift guard/tooling hardening: collector remediation strings, delta-key provenance, the remediation-strings guard, and the transition-window documentation. Expected red on the two canaries inherited from `main` until this PR lands. - **A third PR, still to open** — a confirmed **fail-silent** defect in `scripts/drift-delta.ts`: the delta key omits `scenario`, so same-provider entries collapse and genuinely new drift is downgraded from `block` to `advisory`. Demonstrated on a real run: `block: []` with a new-in-head **critical** sitting in `advisory`, gate passing. Pre-existing, independent of this change, and more serious than anything in either of the other two PRs. ## Recovery record **Pre-strip head: `146cb9d2b81017097480b0616d36b576411aa11c`** (19 commits from the merge-base). The guard commits were pushed to `drift/guard-hardening` **before** this branch was rewritten, and the force-push was lease-pinned to that sha. Nothing was destroyed. This PR is deliberately **not closed**: drift-sync's dedup is open-PR-only, so closing it would re-arm the daily needs-human PR this thread exists to resolve. --- ## Round-4 additions — the classification is now actually tested Review found that the two families this PR exists to classify had **no behavioural coverage at all**: remove them and only the membership checksum reddened. A checksum reports "something changed"; it does not report "these two are excluded and `gpt-live` is not." Four tests now assert what the classification *means*, and they live in the **default** suite — so they gate every PR rather than only running when someone triggers the live drift job: - dropping `gpt-transcribe` reds its own test; dropping `gpt-live-transcribe` reds its own test - a substring-style exclude match reds the boundary on `gpt-live` - a prefix-style match reds it in reverse on `gpt-live-transcribe-mini` That boundary matters concretely: `gpt-live` and `gpt-live-transcribe` are different families, and a substring bug of exactly that shape was found elsewhere in this repo during review (a provider-label fallback resolved `"Gemini Live Transcription session"` to `Transcription`). **The live-canary anti-silence work has been REMOVED from this PR and moved to #349.** An earlier revision of this branch carried an anchor intended to stop the live text canary being silenced. It was defeated at four successive surfaces — the detector function, the gate deciding whether it runs, the call site feeding it, and finally the fetcher the leg calls — each fix closing the named surface and leaving another open one frame further out. One of those defeats passed because the assertion matched a substring that **vitest's own diff output** supplies, so it held whether or not production code produced it. Text-span pinning cannot close that class by extension, so it is out of this PR rather than hardened a fifth time. The replacement — a fetch-stubbed harness that injects a family, drives the real chain, and asserts a report is emitted — belongs in #349, where silencing at any frame fails it. **`FROZEN` extraction is lazy.** It ran at module scope, so one renamed surface threw at import and took out every other pin, every behavioural anchor and the whole frozen-data block with it — 0 of 37 tests running. The obvious repair a future reader reaches for is deleting the offending entry, which would leave everything else silently unguarded. Failures are now contained to their own case. **Live provider canaries no longer run in the offline suite.** `assertNoUnclassifiedFamilies` moved to a plain `text-drift.ts` with no `describe` blocks, so importing it no longer executes live specs. `pnpm test` is 4767 passed / **0 skipped**, and running the full offline suite with a deliberately invalid key produces no request to `api.openai.com`. This leak was **pre-existing** — one offline test already imported the live module before this branch existed — and is only partly this PR's to fix; what is fixed is that nothing in the offline suite reaches the network at this head. Also corrected: the frozen-surface inventory was wrong in both directions, an anchor claim covering 6 of 12 entries was narrowed to what it actually covers, and a test titled "still propagates" now actually calls the function it claims to exercise. **Deferred, deliberately:** other offline regressions still living in `models.drift.ts` run only under `test:drift`. Moving them is a ~440-line change and out of scope here — that kind of accretion is what required this PR to be split in the first place. --- ## Round-5 - **Anti-silence anchor removed** and moved to #349, per the reasoning above. `logic-pin.test.ts` goes 37 → 29 tests. Three pin hashes deleted, **zero added** — nothing was re-pinned on the way out. `text-drift.ts`'s docstring now records that wrapper as *knowingly unguarded pending #349* rather than silently dropping a protection claim. - **The mirror-equivalence guard now catches all three divergences its own docstring names.** Two of the three previously passed when broken — no fixture contained a non-model token, and every output happened to already be sorted, so dropping either protection left the suite green. Two fixtures fix it, each with its own demonstrated failure. - **The four behavioural classification tests still bind**, re-verified on this head: dropping either family reds its own test, a substring match reds on `gpt-live`, a prefix match reds on `gpt-live-transcribe-mini`. **Noted for a follow-up, not done here:** a single `import { expect } from "vitest"` at module scope in `text-drift.ts` is the only thing preventing `scripts/drift-sync.ts` from importing the real module. Replacing it with a plain `Error` carrying the message the collector parses would retire the ~440-line hand-maintained mirror entirely. One line out, 440 deleted. **Also pre-existing, reported not fixed:** the reference scan excludes only `__tests__/drift`, so `isFamilyStillReferenced` returns `true` for never-existing families off fixture literals alone. It errs toward over-blocking a removal, so it is fail-safe rather than dangerous.
Contributor
|
Duplicate of #343, which is now merged (d835753). Both For the record on why this PR existed at all: #343's body was rewritten during review, which deleted the |
jpr5
added a commit
that referenced
this pull request
Aug 4, 2026
Both PR-open steps dedup by matching HTML comments they themselves wrote into a PR BODY — machine state parked inside prose a human owns and edits. On 2026-08-03 a human rewrote PR #343's body wholesale, deleting its markers; ~14h later the scheduled run could no longer find the changeset key on any open PR and opened duplicate PR #350 for the same changeset, which is exactly the spam those guards exist to prevent. Validating-and-failing would have surfaced it a run sooner and STILL left a human to repair prose a human will keep editing, so each step now RE-ASSERTS its own markers before dedup: - ONE `MARKERS` array is the single source for the marker set (`MARKER` seeds it, `NOTE_MARKERS[i]` appends one per committed note). The body writer emits every entry, the repair re-asserts every entry, and the per-note guard matches `NOTE_MARKERS[$i]` — so a marker added there is wired into all three at once. A repair that covered `drift-changeset:` but not `drift-proposal-note:` would leave the second marker's dedup path just as defeatable. - Candidates come from a plain `--state open` listing, never `--search`: that index is body-keyed AND lags an edit by minutes, so it cannot see the body about to be repaired. - Identity is body-INDEPENDENT, since a body-keyed lookup cannot find a PR whose body lost the key. Anchor (a): the head branch now ENDS in the changeset key, and a PR's head branch cannot be renamed — this covers the mixed run whose diff carries no note file. Anchor (b): the PR's changed files include one of this run's note paths — this covers a re-fire whose outcome set changed and so hashes to a DIFFERENT key, where (a) cannot match and the per-note marker is the only thing preventing a duplicate. - The repair is anchor-SCOPED. A PR matched only by a shared note gets that note's marker and nothing else; stamping this run's changeset key into it would assert a changeset it does not carry (in a mixed run, the registry-edit half) and suppress that half forever. - Append-only after the existing body, and a body with every marker intact is left byte-identical: no edit, no API write, no marker duplicated by a re-run. - Repaired PRs are folded into the dedup candidate set, since `--search` cannot see a body edited seconds ago — repairing without that would still open the duplicate the repair just prevented. Fails closed throughout: an unreadable PR list or a failed edit exits non-zero rather than encoding "could not look" as "nothing to repair". No dedup guard is weakened or bypassed.
jpr5
added a commit
that referenced
this pull request
Aug 5, 2026
Both dedup guards match an HTML comment parked in prose a human owns. A wholesale rewrite of PR #343's body deleted its markers and the next scheduled run opened duplicate PR #350. And dedup only listed OPEN PRs, so closing a proposal to reject it guaranteed an identical one every morning after. Each PR-open step now re-asserts its own markers off an anchor a body edit cannot touch, and lists --state all so a rejection is respected.
jpr5
added a commit
that referenced
this pull request
Aug 5, 2026
…et in silence Both PR-open paths tested "a human closed a PR carrying this marker" BEFORE "an open PR already carries it". `--search "<key> in:body"` is state-blind, so one changeset routinely matches several PRs: on 2026-08-04 the live query for eaa8db65f5647493 returned [#350 CLOSED, #343 MERGED], and the closure of the spam DUPLICATE read as a rejection of the changeset itself. The result was permanent suppression with no PR, no Slack (the needs-human alert is gated on `rejected == ''`) and no recovery path — the self-heal lists `--state open` only, so it never touches the closed PR. The open-PR test now runs first, so a closure only counts once no open PR carries the changeset. A merged PR still cannot read as a rejection. And the suppression is no longer silent: `rejected`/`rejected_url` (the ok-applied `rejected` output was previously written and read by nothing at all) feed a new notification naming the closing PR and the two ways back out. Also in the same alerting surface: - A FAILED Slack POST was read as proof of delivery. The catch-all stood down on `alert_*.outcome != 'failure'`, but a `curl -fsS` dying against a rotated webhook exits the step non-zero too — so the likeliest way for alerting to break also disabled the backstop covering it. Delivery is now proven positively with a `posted=true` output written only after the curl returns 0, and every POST is bounded (`--max-time`, a small `--retry`). - The cancellation POST is hoisted ahead of both `always()` artifact uploads, which sat in front of it inside the single 5-minute cancellation budget. - The stale-key preflight keys on a new `unchecked-providers=` MACHINE line instead of grepping drift-sync's human-facing log prose, and an unreadable log (grep exit 2) or an absent line is now a fault rather than proof that every provider was checked.
jpr5
added a commit
that referenced
this pull request
Aug 5, 2026
…the suite
An adversarial pass applied 42 mutations to this workflow: 28 red, 14 GREEN, and
applying all 13 compatible vacuous ones AT ONCE still gave actionlint 0,
`bash -n` clean and 4991/4991 passing. One third of the guards protecting this
PR's fixes could not fail. Each is replaced (not supplemented) by one that can:
- The empty-changeset-key refusal was sliced with
`code.indexOf("\n fi", at)` — a `fi` at 20 spaces of file indent,
which exists nowhere here — so `indexOf` returned -1, `slice(at, -1)` spanned
the whole rest of the step, and 7 (ok-applied) / 14 (needs-human) unrelated
`exit 1`s satisfied the assertion. The refusal could be replaced with `echo
"continuing dedup-blind"` and stay green. Blocks are now sliced on their own
indentation and THROW when the closer is absent.
- The marker self-healer had three independent kill switches: emptying its loop
input, requiring two missing markers rather than one (the #343 -> #350 incident
shape exactly), and writing bare markers its own detector never matches. The
third passed because the assertion ran against the whole step, where the
unrelated PR-BODY writer satisfied it — the wrong artifact.
- The `--search` prohibition was `slice(0, indexOf("gh pr edit"))` plus a
`(?![\s\S]*gh pr edit)` lookahead: a slice that by construction contains no
`gh pr edit`, so the lookahead could never match.
- The closed-PR dedup asserted its literals existed but never that they were
WIRED UP, so pointing the CLOSED selector at the OPEN-narrowed listing, or
dropping `state` from the `--json` list, both passed with the rejection path
dead. It now checks the wiring and EXECUTES the real jq selectors against the
live #350/#343 payload.
- The dropped-notification guard only inspected the missing-webhook branch, so
dropping `-f` (a rotated webhook's 404 then exits 0) or appending `|| true` to
all four POSTs passed.
- `continue-on-error` appeared zero times in the workflow AND the suite, and one
such line on `sync`, `pr` or `alert_gate` defeated three claims. The slicer now
reads it, `simulateJob` models it, and a MODEL guard proves the ban is
load-bearing.
`simulateJob` also no longer equates "selected" with "delivered": an alert whose
`curl` failed delivered nothing, which is what makes the `posted=true` backstop
key testable at all.
And `bot_managed` — the predicate deciding which PRs the self-heal may touch —
had no test anywhere, while the workflow comment asserted "A test pins this
pattern against BOTH branch-construction sites". It now does, by running both
`BRANCH=` builders under bash and feeding the result to the workflow's own jq.
jpr5
added a commit
that referenced
this pull request
Aug 5, 2026
… return, not after it The closed-rejection test moved past TWO gates last round, not one. With the `HEAD_SHA == BASE_SHA` return in front of it, a re-fire whose note is already in the repo (a previous needs-human PR was merged) exited 0 before the rejection was ever tested: no `rejected` output, so the suppression notice could not fire, and the needs-human alert took its contentless "No open PR carries this decision" branch and failed the job — red every morning, on the exact state this work exists to quiet. That gate says nothing about OPEN-vs-CLOSED precedence, so it now runs LAST of the three changeset-keyed decisions. The guard could not see it: it compared the two selectors' text positions, which stayed in the demanded order while the rejection became unreachable. It now constrains the rejection test against EVERY early return in the step — only the open-duplicate dedup may reach `exit 0` first — so a third gate cannot be interposed later either. Also corrects both copies of the reorder comment. Run against the real jq selectors, `[#350 CLOSED, #343 MERGED]` yields REJECTED -> #350 under BOTH orders, because #343 is MERGED and never a pending proposal; only the `[CLOSED, OPEN]` shape changes behaviour, and what makes the merged case recoverable is the suppression notice, not the ordering.
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.
Needs a human decision (drift-sync)
The deterministic, zero-LLM drift-sync found a model-family change it must
NOT auto-apply (a genuinely new/unclassified family, a still-referenced
deprecation, or a registry structural mismatch). It wrote the note file(s)
below and opened this PR so the decision is REACHABLE in the repo.
Note file(s) in this PR
drift-proposals/openai-gpt-live-transcribe-new-family.mddrift-proposals/openai-gpt-transcribe-new-family.mddrift-sync outcome