Fix the two record defects from #52 - #107
Merged
Merged
Conversation
assignat.xlsx.yml said 14 further sheets and listed 15; fifteen is right (18 in the workbook, 3 distinct sheets read). The miscount came from subtracting the four schema.sheets entries rather than the three distinct names, since Data is declared twice. committed_referenced() carried a basename fallback whose condition was a strict superset of the first branch's, so it could only be evaluated where it could not hold — dead code that read as protection. Deleted rather than widened: a basename match would count a same-named file in another directory as referenced and quietly hide orphans. Regression-checked by scanning the same local clones with both versions: audit.json identical apart from the timestamp. See #52. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes two small record defects identified in #52: a miscount in the assignat.xlsx manifest comment and removal of unreachable fallback logic in the audit script that determines whether committed files are referenced by lectures.
Changes:
- Correct the “further sheets” comment in
lectures/assignat.xlsx.ymlfrom 14 to 15 (comment-only; aligns withsheet_count: 18and 3 distinct consumed sheets). - Remove unreachable basename-fallback logic in
scripts/build_audit.py:committed_referenced()and the now-unneededbasevariable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scripts/build_audit.py | Removes dead/unreachable basename fallback branch in committed_referenced() for clearer, correct reference detection logic. |
| lectures/assignat.xlsx.yml | Fixes a comment miscount of unread workbook sheets (14 → 15) to match the enumerated list and workbook sheet count. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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 two small defects recorded in #52, both verified still present on
maintoday.lectures/assignat.xlsx.yml:97— the comment said "14 further sheets" and listed 15. Fifteen is right: 18 sheets in the workbook, 3 distinct sheets read (Budgets,seignor,Data). The miscount came from subtracting the fourschema.sheetsentries rather than the three distinct names, sinceDatais declared twice as two ranges. Comment only;schema.sheet_count: 18was already correct and nothing consumes the number.scripts/build_audit.py,committed_referenced()— the basename fallback's condition (pattern == "local-path" and repo == repo and basename matches) was a strict superset of the first branch's condition, so it could only be evaluated where it could not hold: dead code that read as protection in the function that decides whether a committed file counts as referenced. Deleted rather than widened. A working basename match would count a same-named file in another directory as referenced and quietly hide orphans, which is the opposite of what that function is for.Regression check. Ran
scanover the same local clones with themainversion and this branch's version of the script:audit.jsonis identical apart from thegeneratedtimestamp (40 static files, 24 orphans, 23 live-API lectures in both), and the fixed script runs clean under--strict.See #52.
🤖 Generated with Claude Code