Repository navigation
fix: preserve external benchmark bytes on Windows - #24
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Two unresolved moderate issues remain in traceability validation and report coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Preserves external benchmark bytes on Windows to prevent line-ending conversion from invalidating recorded hashes.
Changes:
- Extends
.gitattributescoverage to external benchmarks. - Adds the INV-006 checkout regression.
- Updates traceability, validation records, recovery guidance, and changelog.
File summaries
| File | Summary |
|---|---|
tests/test_benchmark_checkout.py |
Adds core.autocrlf=true checkout coverage. Moderate issue (3 votes): recorded reports are not included in the regression fixture. |
requirements/traceability.yaml |
Links INV-006 to the implementation and test. Moderate issue (1 vote): the traceability checker does not validate .gitattributes. |
docs/VALIDATION.md |
Records the Windows failure and remediation. |
CHANGELOG.md |
Documents the portability fix. |
benchmarks/external/README.md |
Documents byte preservation and recovery guidance. |
.gitattributes |
Preserves external benchmark bytes across platforms. |
Review details
Suppressed comments (1)
requirements/traceability.yaml:4
- The new
.gitattributesimplementation path is not validated by the traceability gate:scripts/check_traceability.py:9only matches.py,.yaml, and.json, so this entry is silently excluded from missing-file checks. Extend that checker to recognize.gitattributes(with a regression test) or avoid listing an unsupported path, otherwise the traceability check can pass after this implementation file is removed.
implementation: ["src/answerable/domain/serialization.py", ".gitattributes"]
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+17
to
+38
| for source in sources: | ||
| relative = Path("benchmarks/external") / source["path"] | ||
| target = repository / relative | ||
| target.parent.mkdir(parents=True, exist_ok=True) | ||
| shutil.copyfile(root / relative, target) | ||
| subprocess.run(["git", "-c", "core.autocrlf=false", "add", "."], cwd=repository, check=True) | ||
| checkout = tmp_path / "checkout" | ||
| subprocess.run( | ||
| [ | ||
| "git", | ||
| "-c", | ||
| "core.autocrlf=true", | ||
| "checkout-index", | ||
| "--all", | ||
| f"--prefix={checkout.as_posix()}/", | ||
| ], | ||
| cwd=repository, | ||
| check=True, | ||
| ) | ||
| for source in sources: | ||
| data = (checkout / "benchmarks/external" / source["path"]).read_bytes() | ||
| assert hashlib.sha256(data).hexdigest() == source["sha256"] |
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.
Windows quality CI on main failed three external-evaluation tests because Git checkout converted LF snapshots to CRLF, invalidating recorded SHA-256 hashes.
Extend the frozen-benchmark byte-preservation policy to benchmarks/external/**, retaining original source/report bytes and strict hash checking. Add an INV-006 regression using a real Git index and core.autocrlf=true checkout: it failed before the fix and passed afterward. Update traceability, changelog, fresh-clone recovery guidance and the validation record.
Verification completed:
No algorithm, public schema, quality threshold or historical benchmark result changed. Real-agent evaluation remains not run because no authenticated model adapter is available; independent review and pilots remain pending. This repair does not establish stable-product readiness. Rollback: revert this commit.