4.1.4: Implement mode-aware validation and execution gating for Sempai#94
Merged
4.1.4: Implement mode-aware validation and execution gating for Sempai#94
Conversation
Introduce mode-aware Sempai validation to distinguish between parseable and executable rules. Valid 'search' mode rules proceed to normalization placeholder with NOT_IMPLEMENTED, while non-search modes such as 'extract', 'taint', and 'join' fail with E_SEMPAI_UNSUPPORTED_MODE diagnostics. Enhance diagnostics with stable error codes and source span metadata. Add comprehensive unit and behavioural tests covering happy, unhappy, and edge cases. Update docs to reflect new validation boundaries, user-visible behaviour, and mark roadmap item 4.1.4 complete. This change separates parser validation from engine validation, preserving compatibility and deterministic error handling per the ExecPlan 4.1.4 specifications. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Reviewer's GuideImplements mode-aware validation and execution gating in Sempai by extending the YAML model with rule/mode spans and a new ProjectDependsOn search principal, then adding an engine-side post-parse validator that only allows search rules past validation while deterministically erroring on other modes, with corresponding tests and docs updates (including an ExecPlan and roadmap status). Sequence diagram for mode-aware compile_yaml validationsequenceDiagram
actor User
participant Engine
participant SempaiYaml as sempai_yaml_parser
participant ModeValidator as mode_validator
User->>Engine: compile_yaml(yaml)
Engine->>SempaiYaml: parse_rule_file(yaml, None)
SempaiYaml-->>Engine: RuleFile file or DiagnosticReport
alt YAML_parse_error_or_schema_error
SempaiYaml-->>User: DiagnosticReport with E_SEMPAI_YAML_PARSE or E_SEMPAI_SCHEMA_INVALID
else YAML_parsed_successfully
Engine->>ModeValidator: validate_supported_modes(&file)
alt all_rules_have_search_mode
ModeValidator-->>Engine: Ok
Engine-->>User: DiagnosticReport::not_implemented("compile_yaml query-plan normalization")
else first_rule_with_unsupported_mode_found
ModeValidator-->>Engine: Err(DiagnosticReport::validation_error(ESempaiUnsupportedMode, ...))
Engine-->>User: DiagnosticReport with E_SEMPAI_UNSUPPORTED_MODE
end
end
Updated class diagram for Sempai rule model and engine mode gatingclassDiagram
class Engine {
+compile_yaml(yaml: &str) Result~Vec_QueryPlan_ , DiagnosticReport~
-validate_supported_modes(file: &RuleFile) Result~() , DiagnosticReport~
-unsupported_mode_diagnostic(rule: &Rule) Option~DiagnosticReport~
-unsupported_mode_span(rule: &Rule) Option~SourceSpan~
-rule_mode_name(mode: &RuleMode) &str
}
class RuleFile {
+rules() &Vec~Rule~
}
class Rule {
-id: String
-mode: RuleMode
-span: Option~SourceSpan~
-mode_span: Option~SourceSpan~
-message: Option~String~
-languages: Vec~String~
-severity: Option~RuleSeverity~
+id() &String
+rule_span() Option~&SourceSpan~
+mode() &RuleMode
+mode_span() Option~&SourceSpan~
+message() Option~&str~
}
class RuleMode {
<<enumeration>>
Search
Extract
Join
Taint
Other(mode: String)
}
class SearchQueryPrincipal {
<<enumeration>>
Legacy(LegacyFormula)
Match(MatchFormula)
ProjectDependsOn(Value)
}
class RawRule {
+id: Option~Spanned~String~~
+mode: Option~Spanned~String~~
+pattern: Option~Spanned~String~~
+patterns: Option~Spanned~Vec~RawLegacyFormulaObject~~
+pattern_either: Option~Spanned~Vec~RawLegacyFormulaObject~~
+match_formula: Option~Spanned~RawMatchFormula~~
+project_depends_on: Option~Spanned~Value~~
+dest_language: Option~Spanned~String~~
+extract: Option~Spanned~String~~
+taint: Option~Spanned~String~~
+join: Option~Spanned~String~~
}
class DiagnosticReport {
+code: DiagnosticCode
+message: String
+primary_span: Option~SourceSpan~
+notes: Vec~String~
+not_implemented(feature: &str) DiagnosticReport
+validation_error(code: DiagnosticCode, message: String, span: Option~SourceSpan~, notes: Vec~String~) DiagnosticReport
+parser_error(code: DiagnosticCode, message: String, span: Option~SourceSpan~, notes: Vec~String~) DiagnosticReport
}
class DiagnosticCode {
<<enumeration>>
ESempaiYamlParse
ESempaiSchemaInvalid
ESempaiUnsupportedMode
ENotImplemented
}
class SourceSpan {
+start: usize
+end: usize
}
class QueryPlan
Engine --> RuleFile : uses
Engine --> DiagnosticReport : returns
Engine --> DiagnosticCode : uses
Engine --> Rule : validates
RuleFile --> Rule : contains
Rule --> RuleMode : has
Rule --> SearchQueryPrincipal : has
RawRule --> SearchQueryPrincipal : builds
DiagnosticReport --> DiagnosticCode : has
Rule --> SourceSpan : uses
RawRule --> SourceSpan : derives via Spanned
Engine --> SourceSpan : uses for diagnostics
Engine --> QueryPlan : will produce in future
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Mode-Aware Validation and Gating for SempaiThis PR implements deterministic mode-aware validation and execution gating across the parser–engine boundary so only Key ChangesYAML Model & Parser
Engine-Side Validation & Gating
API / Code Hygiene
Tests
Documentation
RationaleMaintain a clean parser vs engine separation: parser validates YAML shape and preserves compatibility keys opaquely; engine enforces mode support and deterministic, first-failure-in-source-order diagnostics. This preserves Semgrep-compatible rules while gating unsupported execution modes at a single well-defined boundary. Risks & Testing
WalkthroughAdd span metadata to parsed rules; preserve Semgrep-compatible Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant Engine as "Engine::compile_yaml"
participant Parser as "YAML Parser"
participant Validator as "Mode Validator"
participant Diagnostic as "Diagnostic Report"
User->>Engine: Supply YAML
Engine->>Parser: parse_rule_file(yaml)
Parser->>Parser: Deserialize rules, capture spans\nParse `project_depends_on` into payload
Parser-->>Engine: Return RuleFile (with rule_span, mode_span)
Engine->>Validator: validate_supported_modes(&file)
Validator->>Validator: Iterate rules, inspect RuleMode
alt mode == Search
Validator-->>Engine: Ok
Engine-->>User: Return NOT_IMPLEMENTED normalization placeholder
else mode != Search
Validator->>Diagnostic: Build ESempaiUnsupportedMode (primary span = mode_span or rule_span)
Diagnostic-->>Engine: Return DiagnosticReport (error)
Engine-->>User: Report failure with targeted span
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…l-project-depends-on' - Add support for opaque 'r2c-internal-project-depends-on' search mode in parser and model. - Introduce source span tracking for rule and mode fields to improve error reporting. - Implement execution gating in Engine::compile_yaml to reject unsupported modes with detailed diagnostics. - Update tests and feature specs to cover dependency search rules and mode validation. - Enhance documentation for new mode-aware validation behavior and error handling. This enables the system to parse dependency search rules compatibly while gating execution support to supported modes only, improving robustness and user feedback. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. crates/sempai/src/tests/engine_tests.rs Comment on file fn compile_yaml_returns_unsupported_mode_for_extract_rules() {
let engine = default_engine();
let result = engine.compile_yaml(concat!(
"rules:\n",
" - id: demo.extract\n",
" mode: extract\n",
" message: extract foo\n",
" languages: [python]\n",
" severity: WARNING\n",
" dest-language: python\n",
" extract: foo($X)\n",
" pattern: source($X)\n",
));
let (code, diag) = first_diagnostic_of_err(result);
assert_eq!(code, DiagnosticCode::ESempaiUnsupportedMode);
assert!(diag.message().contains("extract"));
assert!(diag.primary_span().is_some());
}❌ New issue: Code Duplication |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Refactored compile_yaml_returns_unsupported_mode_for_extract_rules and compile_yaml_returns_unsupported_mode_for_unknown_modes tests to use a new helper function assert_compile_yaml_unsupported_mode. This centralizes code that checks for unsupported mode diagnostics, reduces duplication, and improves test readability. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e302ab400
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/sempai-yaml/src/model.rs`:
- Around line 228-229: Replace the untyped serde_json::Value in the public enum
variant ProjectDependsOn with a dedicated newtype (e.g.,
ProjectDependsOnPayload) and expose accessors/validation on that type;
specifically, create a tuple struct/newtype that wraps the internal Value (or
better, a concrete typed struct), implement/derive Serialize/Deserialize and
TryFrom<Value>/TryInto<Value> or a custom validate/normalize constructor, then
change the enum variant ProjectDependsOn(Value) to
ProjectDependsOn(ProjectDependsOnPayload) and update any
construction/deserialization sites to go through the newtype’s constructor or
TryFrom so future schema changes can validate/normalize without breaking the
public API.
In `@crates/sempai-yaml/src/parser/builders.rs`:
- Around line 238-260: The new project_depends_on branch currently accepts
r2c-internal-project-depends-on for any rule, causing non-search rules (e.g.,
build_extract_rule, build_join_rule, build_taint_rule) to silently drop the
dependency; change the logic in builders.rs so that before returning
SearchQueryPrincipal::ProjectDependsOn for raw.project_depends_on you validate
the rule's mode and reject it with a schema_error when the mode is not Search
(inspect the rule mode field on raw or the surrounding context), and also add
the same mode-specific validation to the other builders (build_extract_rule,
build_join_rule, build_taint_rule) so they either propagate/validate
raw.project_depends_on or surface an error instead of ignoring it; keep existing
error text style and use schema_error like the other branches.
In `@crates/sempai/src/tests/engine_tests.rs`:
- Around line 65-81: The test
compile_yaml_returns_not_implemented_for_project_depends_on_search_rule and the
diagnostic message in engine.rs use US spelling "normalization" but the BDD
expects en-GB "normalisation"; update the user-facing diagnostic strings in
engine.rs (the messages produced where the NotImplemented diagnostic is created
for project-depends/search rule) to use "normalisation" and then change the
assertion in the test
(compile_yaml_returns_not_implemented_for_project_depends_on_search_rule) to
assert that diag.message() contains "normalisation" so both the code and test
match the en-GB spelling.
In `@docs/sempai-query-language-design.md`:
- Around line 429-435: The doc uses inconsistent spelling ("normalisation" vs
"normalization"); update all occurrences to the Oxford -ization form within this
design note so it's consistent—search for and replace instances near the
`sempai::Engine::compile_yaml` implementation note and related mentions of
`NOT_IMPLEMENTED`, `E_SEMPAI_UNSUPPORTED_MODE`, `primary_span`, and `mode` so
the entire paragraph and neighbouring sentences uniformly use "normalization".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 21404e87-1b16-45a6-bcaa-5b681822a30d
📒 Files selected for processing (13)
crates/sempai-yaml/src/model.rscrates/sempai-yaml/src/parser/builders.rscrates/sempai-yaml/src/parser/mod.rscrates/sempai-yaml/src/raw.rscrates/sempai-yaml/src/tests/parser_tests/legacy_tests.rscrates/sempai-yaml/tests/features/sempai_yaml.featurecrates/sempai/src/engine.rscrates/sempai/src/tests/engine_tests.rscrates/sempai/tests/features/sempai_engine.featuredocs/execplans/4-1-4-mode-aware-sempai-validation.mddocs/roadmap.mddocs/sempai-query-language-design.mddocs/users-guide.md
…cy principal - Introduce `ProjectDependsOnPayload` struct with validated `namespace` and `package` fields. - Replace loosely typed `ProjectDependsOn(Value)` with strongly typed payload. - Update parser and builders to deserialize and validate this new payload. - Reject use of `r2c-internal-project-depends-on` in extract, join, and taint modes. - Add unit tests checking valid and invalid usage of dependency principal. - Keep legacy compatibility in search mode for dependency principal. This change improves safety and clarity when handling internal dependency principals in Semgrep YAML rules. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/sempai-yaml/src/model.rs`:
- Around line 233-313: Move the ProjectDependsOnPayload type and its conversion
impls into a new module named project_depends_on: create a new module file
containing the struct ProjectDependsOnPayload, its methods (namespace, package,
into_inner), the impl TryFrom<Value> for ProjectDependsOnPayload and impl
From<ProjectDependsOnPayload> for Value, keeping the exact logic; then in the
original model module remove the type and conversion impls and re-export it with
a pub mod declaration and a pub use project_depends_on::ProjectDependsOnPayload
so the type remains available at the original module surface. Ensure the new
module has the same imports (serde derives and Value) so the conversions
compile.
In `@crates/sempai-yaml/src/tests/parser_tests/legacy_tests.rs`:
- Around line 55-91: Replace the two nearly identical tests
parse_project_depends_on_with_legacy_principal_fails and
parse_project_depends_on_requires_namespace_and_package with a single
parameterized rstest that enumerates both YAML inputs and their expected error
substrings; call first_err_diagnostic for each case and assert the
DiagnosticCode::ESempaiSchemaInvalid and that the returned message contains the
expected substring, keeping has_span assertions; ensure the new test references
the original YAML samples (or strings) and the helper first_err_diagnostic to
preserve behavior.
In `@docs/execplans/4-1-4-mode-aware-sempai-validation.md`:
- Around line 264-266: Update the doc's retrospective variant type for
r2c-internal-project-depends-on so it matches the implemented API: change the
opaque `SearchQueryPrincipal::ProjectDependsOn(serde_json::Value)` reference to
use the actual typed payload name used in code (e.g.
`SearchQueryPrincipal::ProjectDependsOn(ProjectDependsOnPayload)`), ensuring the
variant signature in the docs matches the real type returned/accepted by the
ProjectDependsOn implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 917fd672-17d8-4104-ac13-62264651b2a1
📒 Files selected for processing (11)
crates/sempai-yaml/src/lib.rscrates/sempai-yaml/src/model.rscrates/sempai-yaml/src/parser/builders.rscrates/sempai-yaml/src/tests/parser_tests/legacy_tests.rscrates/sempai-yaml/src/tests/parser_tests/mode_tests.rscrates/sempai/src/engine.rscrates/sempai/src/lib.rscrates/sempai/src/mode_validation.rscrates/sempai/tests/features/sempai_engine.featuredocs/execplans/4-1-4-mode-aware-sempai-validation.mddocs/sempai-query-language-design.md
Owner
Author
|
@coderabbitai Have the following now been resolved? Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity. |
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. crates/sempai-yaml/src/tests/parser_tests/legacy_tests.rs Comment on file fn parse_project_depends_on_with_legacy_principal_fails() {
let yaml = concat!(
"rules:\n",
" - id: demo.depends\n",
" message: detect vulnerable dependency\n",
" languages: [python]\n",
" severity: WARNING\n",
" pattern: foo()\n",
" r2c-internal-project-depends-on:\n",
" namespace: pypi\n",
" package: requests\n",
);
let (code, message, has_span) = first_err_diagnostic(yaml);
assert_eq!(code, DiagnosticCode::ESempaiSchemaInvalid);
assert!(message.contains("exactly one top-level query principal"));
assert!(has_span);
}❌ New issue: Code Duplication |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 21726
No requirements are annulled: neither suggestion violates en-GB-oxendict spelling ("behaviour-driven development" is already correct en-GB; "domain-specific language" has no spelling conflict), and neither increases cyclomatic complexity. |
This comment was marked as resolved.
This comment was marked as resolved.
Moved the ProjectDependsOnPayload struct and its implementations from model.rs into a new dedicated module project_depends_on.rs. Updated model.rs to re-export ProjectDependsOnPayload and reference the new module. This improves code organization and separation of concerns in the sempai-yaml crate. Also updated related tests to use the new module properly and adapted execplan documentation to reflect the new type usage. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…d helper Replaced rstest parameterized cases in legacy_tests.rs with individual test functions that use the assert_schema_invalid helper for clearer and more direct test assertions. This simplifies test code and improves diagnostic message checking with explicit assert messages. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/sempai-yaml/src/tests/parser_tests/legacy_tests.rs (1)
65-97: 🛠️ Refactor suggestion | 🟠 MajorReplace duplicated negative tests with one parameterized
#[rstest].Collapse Line 66 and Line 84 into one
#[rstest]with two#[case]inputs, and keepassert_schema_invalid(...)as the shared assertion path.♻️ Proposed refactor
-#[test] -fn parse_project_depends_on_with_legacy_principal_fails() { - assert_schema_invalid( - concat!( - "rules:\n", - " - id: demo.depends\n", - " message: detect vulnerable dependency\n", - " languages: [python]\n", - " severity: WARNING\n", - " pattern: foo()\n", - " r2c-internal-project-depends-on:\n", - " namespace: pypi\n", - " package: requests\n", - ), - "exactly one top-level query principal", - ); -} - -#[test] -fn parse_project_depends_on_requires_namespace_and_package() { - assert_schema_invalid( - concat!( - "rules:\n", - " - id: demo.depends.invalid\n", - " message: detect vulnerable dependency\n", - " languages: [python]\n", - " severity: WARNING\n", - " r2c-internal-project-depends-on:\n", - " namespace: pypi\n", - ), - "must define string `namespace` and `package` fields", - ); -} +#[rstest] +#[case::with_legacy_principal( + concat!( + "rules:\n", + " - id: demo.depends\n", + " message: detect vulnerable dependency\n", + " languages: [python]\n", + " severity: WARNING\n", + " pattern: foo()\n", + " r2c-internal-project-depends-on:\n", + " namespace: pypi\n", + " package: requests\n", + ), + "exactly one top-level query principal", +)] +#[case::missing_package( + concat!( + "rules:\n", + " - id: demo.depends.invalid\n", + " message: detect vulnerable dependency\n", + " languages: [python]\n", + " severity: WARNING\n", + " r2c-internal-project-depends-on:\n", + " namespace: pypi\n", + ), + "must define string `namespace` and `package` fields", +)] +fn reject_invalid_project_depends_on(#[case] yaml: &str, #[case] expected_fragment: &str) { + assert_schema_invalid(yaml, expected_fragment); +}As per coding guidelines, "Replace duplicated tests with
#[rstest(...)]parameterised cases in Rust."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/sempai-yaml/src/tests/parser_tests/legacy_tests.rs` around lines 65 - 97, Replace the two duplicated tests parse_project_depends_on_with_legacy_principal_fails and parse_project_depends_on_requires_namespace_and_package with a single parameterized rstest: add #[rstest] on a new test fn (e.g., parse_project_depends_on_invalid_cases) and supply two #[case] inputs containing the two different YAML payloads and their respective expected error strings ("exactly one top-level query principal" and "must define string `namespace` and `package` fields"); inside the test call assert_schema_invalid(case_yaml, case_expected) for the shared assertion path and ensure the rstest crate is imported/enabled for the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/sempai-yaml/src/tests/parser_tests/legacy_tests.rs`:
- Around line 65-97: Replace the two duplicated tests
parse_project_depends_on_with_legacy_principal_fails and
parse_project_depends_on_requires_namespace_and_package with a single
parameterized rstest: add #[rstest] on a new test fn (e.g.,
parse_project_depends_on_invalid_cases) and supply two #[case] inputs containing
the two different YAML payloads and their respective expected error strings
("exactly one top-level query principal" and "must define string `namespace` and
`package` fields"); inside the test call assert_schema_invalid(case_yaml,
case_expected) for the shared assertion path and ensure the rstest crate is
imported/enabled for the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7a367078-2f1c-4f4d-a926-b3df69184d8f
📒 Files selected for processing (4)
crates/sempai-yaml/src/model.rscrates/sempai-yaml/src/project_depends_on.rscrates/sempai-yaml/src/tests/parser_tests/legacy_tests.rsdocs/execplans/4-1-4-mode-aware-sempai-validation.md
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. crates/sempai-yaml/src/tests/parser_tests/legacy_tests.rs Comment on file fn parse_project_depends_on_with_legacy_principal_fails() {
assert_schema_invalid(
concat!(
"rules:\n",
" - id: demo.depends\n",
" message: detect vulnerable dependency\n",
" languages: [python]\n",
" severity: WARNING\n",
" pattern: foo()\n",
" r2c-internal-project-depends-on:\n",
" namespace: pypi\n",
" package: requests\n",
),
"exactly one top-level query principal",
);
}❌ New issue: Code Duplication |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5425
A further merge with File: Fix (if you prefer to act on the finding)Merge the two tests into a single #[rstest]
#[case::with_legacy_principal(
concat!(
"rules:\n",
" - id: demo.depends\n",
" message: detect vulnerable dependency\n",
" languages: [python]\n",
" severity: WARNING\n",
" pattern: foo()\n",
" r2c-internal-project-depends-on:\n",
" namespace: pypi\n",
" package: requests\n",
),
"exactly one top-level query principal",
)]
#[case::missing_required_fields(
concat!(
"rules:\n",
" - id: demo.depends.invalid\n",
" message: detect vulnerable dependency\n",
" languages: [python]\n",
" severity: WARNING\n",
" r2c-internal-project-depends-on:\n",
" namespace: pypi\n",
),
"must define string `namespace` and `package` fields",
)]
fn reject_invalid_project_depends_on(#[case] yaml: &str, #[case] expected_fragment: &str) {
assert_schema_invalid(yaml, expected_fragment);
}Remove the original AI Coding Agent PromptSuppression message (if you prefer not to refactor further)
|
…dation tests Refactored existing legacy_tests.rs to use rstest parameterized test cases for invalid 'r2c-internal-project-depends-on' rules. This replaces multiple individual test functions with a single parameterized test covering both legacy principal conflicts and missing required fields, improving test maintainability and clarity. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
5 tasks
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
1 participant
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.
Summary
searchrules proceed past validation; non-search modes (extract,taint,join, and unknown modes) produce deterministic errors.r2c-internal-project-depends-onfor search mode validation.Changes
r2c-internal-project-depends-onprincipal and updated error messaging when multiple principals are defined.project_depends_onas part of search/legacy field checks.r2c-internal-project-depends-oninto the RawRule.compile_yaml(...)mode-aware behavior and diagnostic expectations.Rationale
Testing plan
r2c-internal-project-depends-onas a compatibility key (ProjectDependsOn principal).compile_yaml(...)returns a not-implemented placeholder for validsearchrules but with mode-aware validation for non-search modes that fail withE_SEMPAI_UNSUPPORTED_MODE.Documentation plan
compile_yaml(...).Risks and constraints
Context
Summary by Sourcery
Documentation: - Add a detailed ExecPlan documenting how Sempai will perform mode-aware validation and execution gating, including constraints, risks, progress tracking, and test and rollout strategy for milestone 4.1.4.
Summary by Sourcery
Introduce mode-aware validation and execution gating for Sempai YAML compilation, while preserving compatibility for dependency search rules and updating documentation and tests accordingly.
New Features:
r2c-internal-project-depends-onas a search principal.Enhancements:
compile_yamlon supported modes, allowing onlysearchrules to proceed and rejecting other modes with deterministic diagnostics.match, orr2c-internal-project-depends-onis defined.Documentation:
compile_yamlbehaviour and diagnostics in the user guide and design document, including the parser–engine validation boundary and handling of dependency rules.Tests:
📎 Task: https://www.devboxer.com/task/541762c5-fca6-442c-ad49-410edc3daa24