fix(hooks): parse provider-prefixed and bracket-suffixed model ids in provenance footer#664
Open
tractorjuice wants to merge 3 commits into
Open
fix(hooks): parse provider-prefixed and bracket-suffixed model ids in provenance footer#664tractorjuice wants to merge 3 commits into
tractorjuice wants to merge 3 commits into
Conversation
…e-model.mjs provenance-stamp.mjs self-executes and exports nothing, so its pure model and effort helpers could not be unit-tested by import. Extract them into an importable leaf module, mirroring the existing session-nudge.mjs (pure) vs session-learner.mjs (hook) split. Behaviour-preserving: the regex and the 4-row effort matrix are unchanged. The characterization suite locks current behaviour, including the known provider-prefixed-id bug, which is fixed separately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GNtKcGCC1wcQDErJZRkyMc
… provenance footer The model-id character class was [a-z0-9.-], so any footer id containing a provider prefix or a context-window suffix parsed as null and the model was silently dropped from the Build Provenance block. This is not hypothetical: the current id in use, claude-opus-4-8[1m], is rejected by the old class today. Widened to cover / : [ ] alongside the existing dot and hyphen, which also admits moonshotai/kimi-v3 and bedrock-style us.anthropic.claude-* ids. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GNtKcGCC1wcQDErJZRkyMc
…e-only Documentation-as-code, not a runtime fix: xhigh is the top EFFORT_RANK, so a cap of xhigh never triggers a downgrade and the row behaves identically to the model being absent. Added for explicitness, and as 'xhigh' rather than 'max' so a requested xhigh is not wrongly downgraded. Also records the Claude-only constraint on the matrix in a comment, so no one adds a non-Claude id (the hook never runs outside Claude Code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GNtKcGCC1wcQDErJZRkyMc
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.
Summary
Fixes a silent data-loss bug in the Build Provenance stamp, and makes the model/effort logic unit-testable for the first time.
extractModelFromContent()matched model ids against[a-z0-9.-]+. Any id containing a provider prefix or a context-window suffix failed the match and returnednull, so the model was silently omitted from the provenance block.This is not hypothetical. The id currently in use,
claude-opus-4-8[1m], is rejected by the old character class today.Changes
Three commits, each independently reviewable:
refactor(hooks)— extract the pure model/effort helpers out ofprovenance-stamp.mjsinto a new importableprovenance-model.mjs. The hook self-executes and callsprocess.exit(0)at top level, so its helpers could not be imported by a test without running the whole hook. Mirrors the existingsession-nudge.mjs(pure) vssession-learner.mjs(hook) split. Behaviour-preserving.fix(hooks)— widen the model-id character class to cover/ : [ ]alongside the existing dot and hyphen. Admitsclaude-opus-4-8[1m],moonshotai/kimi-k3, and bedrock-styleus.anthropic.claude-*.chore(hooks)— add the missingclaude-opus-4-8row toMODEL_MAX_EFFORTand record the Claude-only constraint in a comment.On commit 3, honestly
This row is documentation-as-code, not a runtime fix.
xhighis the topEFFORT_RANK, so a cap ofxhighnever triggers a downgrade and behaves identically to the model being absent from the matrix. It is added for explicitness, and as'xhigh'rather than'max'so a requestedxhighis not wrongly downgraded.Out of scope, deliberately
EFFORT_RANKranksxhigh(4) abovemax(3), butCLAUDE.mddocumentsmaxas the deepest tier abovexhigh. That discrepancy is real and is left untouched here — it is a separate question about actual harness behaviour and deserves its own issue rather than being silently "fixed" wrong inside a parsing PR.Testing
New
tests/plugin/provenance-model.test.mjs, 14 cases, written test-first: the characterization suite locked current behaviour (including the bug, asserted asnull) before extraction, then the baseline assertion was flipped when the fix landed.node --test tests/plugin/*.test.mjs— 107 pass, 0 failnode --test tests/plugin/test_*.mjs— 92 pass, 0 failtest_provenance_okf_frontmatter.mjsstill green, confirming the refactor changed no hook behaviourAuto-discovered by CI via the existing
tests/plugin/*.test.mjsglob.🤖 Generated with Claude Code