core: reject reserved factor names (rep, error, verdict, …) instead of corrupting the statistics - #35
Open
ChrisW09 wants to merge 1 commit into
Open
core: reject reserved factor names (rep, error, verdict, …) instead of corrupting the statistics#35ChrisW09 wants to merge 1 commit into
ChrisW09 wants to merge 1 commit into
Conversation
The stats layer looks up columns by factor name, but to_records renames
factors that collide with bookkeeping columns (input_id, rep, judge_rep,
verdict, reasoning, error) to '<name>_factor'. The lookup then resolves
to the bookkeeping column instead: a factor named 'rep' crashes
analysis_frame ('cannot insert rep, already exists'), and a factor named
'error' silently groups configs together and erases real effects (a
5-vs-2 effect vanished into 'no variance in verdict').
Validate at both entry points: Factor.__post_init__ (study definition)
and Ratings.__post_init__ (library mode), with a clear error naming the
reserved set.
Fixes #5
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes #5.
Ratings.to_records()renames factors colliding with bookkeeping columns to<name>_factor, but every stats consumer still looks up the original name fromratings.factors— which then resolves to the bookkeeping column. Verified consequences: a factor named"rep"(or"input_id","verdict") crashesanalysis_framewithValueError: cannot insert rep, already exists; a factor named"error"silently merges configs per question and a real 5-vs-2 effect vanishes into "skipped (no variance in verdict)".Rather than threading the rename map through five stats modules, this rejects reserved names up front — the issue's suggested cheapest fix:
Factor.__post_init__raises with a clear message naming the reserved set (study-definition path),Ratings.__post_init__raises identically (library-mode path for directly builtRatings),to_recordsrename stays as a last-ditch guard.The reserved tuple is declared as
RESERVED_FACTOR_NAMESinstudy.pywith cross-reference comments toRatings._RESERVED(kept literal there to avoid an import cycle).Test: full cafe-core suite — 109 passed (the 1 failure is pre-existing #4, fix in PR #34).
🤖 Generated with Claude Code