Conversation
…el.rs Move project-level cross-file validation (AGM-006, XP-004/005/006, VER-001) from pipeline.rs into its own module. This is pure code motion with no behavioral changes - the function signature changes from `fn` to `pub(crate) fn` to allow the import from pipeline.rs, and the private `join_paths` helper moves alongside it. Six unit tests that directly exercise run_project_level_checks are co-located with the extracted code. Part of #474.
- Use file-level #![cfg(feature = "filesystem")] instead of per-item
attributes on all 7 imports and 2 functions
- Fix misleading join_paths doc comment (Cow avoids one intermediate
String per path, not all heap allocation)
- Flatten double-nested if for XP-004/005/006 guard into a single
compound condition
- Add comment documenting intentional silent-skip when XP-004 is
disabled and a file read fails (XP-004 owns the read-error diagnostic)
- Add test_join_paths_{empty,single,multiple} for the private helper
- Add test_agm006_message_variants covering nested vs. sibling variants
- Add test_ver001_uses_agnix_toml_path_when_present covering the
.agnix.toml report path branch
- Add test_xp004_disabled_xp005_enabled_silent_skip documenting the
silent-skip behavior for XP-005/006 when XP-004 is disabled
- Remove duplicate test_xp004_enabled_still_emits_read_error (covered
by test_xp004_read_error_for_missing_instruction_file)
- Flatten AGM-006 nested if into a single compound guard - Add comment explaining the file-level #![cfg(feature = "filesystem")] choice vs. per-item attributes - Extend test_agm006_message_variants to assert listed paths in messages - Extend test_ver001_disabled_skips_diagnostics to assert root_dir fallback when .agnix.toml is absent - Add test_ver001_suppressed_when_version_pinned: VER-001 should not fire when at least one tool version is pinned in config - Add test_xp_single_instruction_file_no_diagnostics: XP-004/005/006 guard requires len() > 1 instruction files
- Update Validation Flow diagram in CLAUDE.md and AGENTS.md: rename 'Post-processing' step to 'Project-level checks', add VER-001 to the rule list, and reference rules/project_level module - Update Validator::metadata() doc comment in rules/mod.rs: rename 'Pipeline-level post-processing rules' to 'Project-level cross-file rules' and point to rules::project_level CLAUDE.md and AGENTS.md remain byte-for-byte identical.
Summary of ChangesHello @avifenesh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the core validation pipeline by extracting project-level validation rules into a new, dedicated module. This change improves the modularity and organization of the codebase, aligning cross-file analysis rules with the existing architecture for other validators. The primary goal is to reduce the complexity of the main pipeline file without altering any external behavior or public APIs. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The refactoring successfully decouples project-level validation logic from the pipeline orchestration, moving it into a dedicated rules/project_level.rs module. This improves maintainability and follows the existing architectural pattern where rule logic resides in the rules/ directory. The extracted code is well-structured, and the addition of new unit tests for edge cases like path joining and version pinning is excellent. I have provided feedback regarding the externalization of hardcoded strings to the existing i18n system for consistency with the rest of the codebase.
There was a problem hiding this comment.
Pull request overview
Refactors agnix-core project-level (cross-file) validation by moving the run_project_level_checks logic out of the orchestration layer (pipeline.rs) into a dedicated rules module, aligning architecture with the rest of the validators/rules structure.
Changes:
- Added
crates/agnix-core/src/rules/project_level.rscontaining the extracted project-level checks (AGM-006, XP-004/005/006, VER-001) plus unit tests. - Updated
pipeline.rsto import and callrules::project_level::run_project_level_checks, removing the inlined implementation and its tests. - Updated
rules/mod.rsand documentation (CLAUDE.md,AGENTS.md) to reference the new module/location.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/agnix-core/src/rules/project_level.rs | New module housing extracted cross-file checks and associated unit tests. |
| crates/agnix-core/src/rules/mod.rs | Exposes the new project_level module and updates validator metadata docs. |
| crates/agnix-core/src/pipeline.rs | Removes extracted logic/tests and wires pipeline to call the new module function. |
| CLAUDE.md | Updates validation flow diagram to reference project-level checks module. |
| AGENTS.md | Updates validation flow diagram to reference project-level checks module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
avifenesh
left a comment
There was a problem hiding this comment.
Thank you for the reviews! Addressing all comments:
Gemini (XP-004/005/006 i18n): Fixed in ca99a01 - replaced all hardcoded diagnostic messages and suggestions with t! macro calls using the existing localization keys (rules.xp_004.message, rules.xp_004.suggestion, rules.xp_005.message, rules.xp_005.suggestion, rules.xp_006.suggestion).
Copilot (duplicate LintConfig import, line 272): use super::* in Rust does not re-export the parent module's private use imports - it only brings in public items and functions defined in the parent. Since use crate::config::LintConfig in the parent is a private import (not pub use), the use crate::config::LintConfig in the test module is required for LintConfig to be accessible. The CI test suite compiles and passes on all platforms with no E0252 error. However, to address the concern clearly and reduce ambiguity, removing the parent-level import and keeping only the test-level one is the simplest fix.
Copilot (hardcoded paths in join_paths tests, lines 283/293): Valid point. While Windows CI passes (Windows Path::new("/foo/bar.md").to_string_lossy() returns the same string), explicitly constructing expected values from the paths is better practice. Fixing now.
Build expected strings from path.to_string_lossy() rather than hardcoded forward-slash strings, ensuring the assertions work correctly on Windows where path rendering may differ.
Summary
run_project_level_checks(~222 lines) and itsjoin_pathshelper frompipeline.rsinto a newcrates/agnix-core/src/rules/project_level.rsmodulerules/alongside all other validators, consistent with the existing architecturepipeline.rsis reduced from ~1736 to ~1226 lines; orchestration and rule logic are no longer conflatedChanges
crates/agnix-core/src/rules/project_level.rs- extracted code withpub(crate) fn run_project_level_checks, file-level#![cfg(feature = "filesystem")], and 13 unit testscrates/agnix-core/src/rules/mod.rs-pub mod project_leveldeclaration; updatedValidator::metadata()doc comment to referencerules::project_levelcrates/agnix-core/src/pipeline.rs-use crate::rules::project_level::run_project_level_checksimport; removed ~280 lines of moved code and ~230 lines of moved testsCLAUDE.md/AGENTS.md- Validation Flow diagram updated to referencerules/project_leveland include VER-001No behavioral changes
This is a pure code-motion refactoring. The function signature, behavior, and all public APIs (
validate_project,validate_project_rules,validate_project_with_registry) are unchanged. Thecrates/agnix-core/tests/api_contract.rstests confirm the public surface is stable.Tests
cargo test --features filesystem)pipeline.rsto their new co-located modulejoin_pathsvariants, AGM-006 message variants (with path assertions), VER-001.agnix.tomlpath, VER-001 root_dir fallback, VER-001 version-pinned suppression, XP single-file early-exit guardRelated
Closes #474