Conversation
Add a coverage check to `validate-manifest`: parse the header of every `.lean` file under `LeanEval/` and fail unless each one is named by some manifest `module` field or imported, directly or transitively, by a module that is. The manifest is CI's only entry point into the problem sources, so an unreached module is never compiled and never validated. Also reject non-`.toml` files in `manifests/problems/` instead of skipping them silently, and reject manifest entries whose `module` names no file. Fixes #519. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhJsJCk5YGmsdcbz1fWT8y
Three gaps in the coverage check, from review: Flattening a module to dot-joined text conflated distinct files. `LeanEval/Foo/Bar.lean` and `LeanEval/Foo.Bar.lean` both read as `LeanEval.Foo.Bar`, so a manifest entry for the nested file vouched for a sibling Lake never builds; conversely `import LeanEval.«Foo-Bar»` did not match `LeanEval/Foo-Bar.lean`. Key on `Lean.Name` throughout, built from path components, and read manifest `module` fields with the shared `parseModuleName` the inventory executable now also uses. Skipping every dotfile in `manifests/problems/` reopened the hole it was meant to close: the `@[eval_problem]` elaborator reads every `*.toml` there, so a hidden `.foo.toml` could claim a tagged declaration that never reached the catalog. Exempt `.DS_Store` alone. Reject symlinks under `LeanEval/` rather than following them: a directory link duplicates a subtree under a second module prefix, or points at an ancestor and recurses forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhJsJCk5YGmsdcbz1fWT8y
Collaborator
Author
|
Second-opinion review (Codex) turned up three real gaps, all now fixed in 28332a0:
Tests grew to 13, covering each case. |
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.
This PR makes
validate-manifestfail on any problem module the manifest cannot reach. It parses the header of every.leanfile underLeanEval/and rejects the ones that are neither named by some manifestmodulefield nor imported, directly or transitively, by a module that is.manifests/problems/*.tomlis CI's only entry point into the problem sources, so an unreached module is never compiled and never validated: a new problem can be added with a broken statement and CI stays green. The check parses imports rather than building anything, so it costs no build time and runs before the inventory cross-check.Two smaller guards come with it.
loadManifestnow rejects non-.tomlfiles inmanifests/problems/instead of skipping them silently, which is what let #513 land a manifest nameddirect_summand.leanand take its module out of the build with it; dotfiles are still ignored so a stray.DS_Storecannot wedge CI. And a manifest entry whosemodulenames no file underLeanEval/is now reported as a typo rather than surfacing later as an unrelated-looking unreachable-module error.mainpasses the new check as-is: 245 manifest entries name 229 of the 237 modules underLeanEval/, and the other 8 are imported by modules that are.Fixes #519.
🤖 Prepared with Claude Code