fix(drift): route every provider surface to auto-fix via a surface registry#311
Merged
Conversation
…gistry extractProviderName matched drift-block titles against ~9 hardcoded PROVIDER_MAP keys via text.includes(). ~15 additional provider surfaces (Cohere, Bedrock, Ollama, fal, ElevenLabs, images, video, moderation, transcription, rerank, vertex-ai, ...) emit valid, fully-parseable drift blocks whose PROSE titles are not keys, so extractProviderName returned null, the block was quarantined (exit 5), and fix-drift.yml (which gates auto-remediation on exit 2) never ran for them. Replace the title-substring match with structural keying: - New single-source-of-truth SURFACE_REGISTRY (surface-registry.ts), slug-keyed, imported by BOTH schema.ts (validate a slug at emit time) and the collector (resolve slug -> builderFile/functions). - formatDriftReport gains an optional `surface` slug that emits a machine-readable `Surface: <slug>` marker line; an unknown slug throws at emit time. - Collector resolution: marker present + known slug -> exit-2 entry (auto-fixable); marker present + UNKNOWN slug -> THROW (loud, exit 1), never silent quarantine; no-marker legacy block -> today's provider-label fallback (exit 5 only for truly un-attributable output). - Wire every currently-unmapped *.drift.ts emitter to pass its slug; builderFunctions filled from the real source exports. - Registry-coverage test asserts every entry resolves to an existing builderFile with non-empty builderFunctions. models.drift.ts is left as-is (deliberate PROVIDER_MAP-matching canaries).
The collector wrote only { timestamp, entries, quarantine? }, but the
base-report reuse guard reads report.generatedAt and report.conclusion
(isBaseReportReusable requires a same-UTC-day generatedAt plus a known-good
conclusion). Since neither field was ever written, reuse always failed and
every PR ran a fresh ~30-min live base.
Write both fields on the report:
- generatedAt: ISO alias of the existing timestamp (timestamp kept for
back-compat with existing consumers).
- conclusion: derived from the collector exit code via conclusionForExitCode
(0 -> "clean", 2 -> "critical", 5 -> "quarantine").
The exit code is now computed before the report write so conclusion can be
derived from it. Efficiency only — a failed reuse just runs a fresh base, so
safety is unaffected.
commit: |
…r registration The collector indexed SURFACE_REGISTRY[surfaceKey] with untrusted marker text. A plain-object bracket lookup walks the prototype chain, so a slug like constructor/toString/hasOwnProperty/valueOf/__proto__ resolved to a truthy inherited member, skipped the unknown-surface throw, and emitted a garbage DriftEntry (builderFile: undefined) that fix-drift.ts chokes on far from the root cause. Gate the lookup on isKnownSurface (Object.prototype.hasOwnProperty .call) before indexing, matching the emit-side guard, so an unknown or prototype-member slug THROWS loudly. Apply the same Object.hasOwn discipline to the legacy PROVIDER_LABEL_MAP[label] fallback for parity. Strengthen the WS-5 red-green so >=3 cells genuinely require the surface seam. The prior cases (fal-sync, elevenlabs) stayed green on revert because their titles contained legacy provider-label substrings that extractProviderName matched anyway. Replace with four surfaces (moderation, video, transcription, rerank) whose emitted titles contain NO legacy label, so the marker is the only route; each asserts extractProviderName is null and goes RED when the seam is reverted. Replace the tautological coverage test (which iterated the registry's own keys) with a static TypeScript-AST scan of every *.drift.ts formatDriftReport third argument, asserting each emitted slug is registered (and no orphan slugs). This locks the every-emitter-is-registered invariant at TEST time, so an unregistered emitter fails CI even on a credential-less run where the runtime throw is never reached. Add a generatedAt->sameUtcDay staleness test proving a prior-day base report is not reusable. Keep the existsSync builder-file integrity test.
jpr5
marked this pull request as ready for review
July 17, 2026 20:30
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.
The hole
extractProviderNamematched drift-block titles against ~9 hardcodedPROVIDER_MAPkeys viatext.includes(). ~15 additional provider surfaces(Cohere, Bedrock, Ollama, fal, ElevenLabs, images, video, moderation,
transcription, rerank, vertex-ai, ...) emit valid, fully-parseable
API DRIFT DETECTED:blocks whose PROSE titles are not keys →extractProviderNamereturns null → the block is quarantined (exit 5) →
fix-drift.ymlgatesauto-remediation on exit 2 only, so those surfaces were never auto-fixed
(mislabeled "manual triage").
models.drift.tsis left as-is (deliberatePROVIDER_MAP-matching canaries).
The design — structural keying via a single-source-of-truth registry
SURFACE_REGISTRY(src/__tests__/drift/surface-registry.ts), slug-keyed,imported by BOTH
schema.ts(validate a slug at emit time) and the collector(resolve slug → builderFile/builderFunctions). One table, two consumers —
adding a surface is a single edit both see.
formatDriftReport(context, diffs, surface?)emits a machine-readableSurface: <slug>marker line. An unknown slug throws at emit time.quarantine — a future unkeyed surface fails loudly instead of silently
routing to manual triage.
truly un-attributable output).
*.drift.tsemitter now passes its slug;builderFunctionsfilled from the real source exports (not invented).Bedrock is split per the spec:
bedrock-invoke/bedrock-invoke-stream→bedrock.ts,bedrock-converse/bedrock-converse-stream→bedrock-converse.ts. Vertex AI keys onto the Gemini path (gemini.ts).builderFilewith non-emptybuilderFunctions(mirrorsfix-drift.tsvalidation, so a bad entry fails locally, not in CI).
Base-report reuse fix (own commit — efficiency, not safety)
The collector wrote only
{ timestamp, entries, quarantine? }, but the reuseguard reads
report.generatedAt+report.conclusion, so reuse alwaysfailed and every PR ran a fresh ~30-min live base. The collector now writes
generatedAt(ISO alias oftimestamp, kept for back-compat) andconclusion(derived from the exit code: 0→clean, 2→critical, 5→quarantine).
Red-green (fixtures only, no live SDK)
RED (original code) — a Cohere/fal/ElevenLabs drift block routes to
quarantine(len 1, exit 5), never an entry:GREEN (new code, with the
Surface:marker) — all three route to anexit-2 entry,
quarantineempty:Surfaces flipped 5 → 2:
cohere-chat,fal-sync,elevenlabs(plus everyother previously-unmapped surface: rerank, bedrock-*, ollama, images, video,
moderation, transcription, vertex-ai, fal-queue).
Additional in-suite coverage: unknown-slug marker throws (not quarantine);
isBaseReportReusableaccepts a written clean report; registry-coverage test.Full suite: 4637 passed.
format:check,lint,tsc(both tsconfigs),build,commitlintall clean.🤖 Generated with Claude Code