feat(sources): add local json adapter - #16
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5ce6f364d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) -> ExperimentGroup: | ||
| """Load one supported local JSON format into an ExperimentGroup.""" | ||
| with path.open(encoding="utf-8") as file: | ||
| data = json.load(file) |
There was a problem hiding this comment.
Reject non-finite constants before validating metrics
When a local file contains Python-style NaN, Infinity, or -Infinity as a metric value, json.load accepts it even though it is not valid JSON, and the strict Pydantic float fields also admit the resulting non-finite value. The resulting canonical model cannot round-trip faithfully—for example, Pydantic's default model_dump_json() serializes NaN as null—so ingestion silently changes experiment evidence instead of reporting malformed input. Reject these constants during parsing or explicitly validate all metric floats as finite.
Useful? React with 👍 / 👎.
Problem
What situation existed before this change? Why did it need to change?
Change
What did you actually do? Summarize the change, not the diff line-by-line.
Design Decisions
Any non-obvious choices made while implementing this — trade-offs, rejected
alternatives, or anything a future reader would ask "why did you do it this
way?" about.
Validation
pytestpassesruff check .passesmypy srcpassesRisks
What could this break? What wasn't tested? What assumptions might not hold
for other inputs or backends?
Related