Skip to content

Feat(validation): Add support for Prefix keys - #93

Open
ClausHolbechArista wants to merge 24 commits into
aristanetworks:mainfrom
ClausHolbechArista:prefix-keys
Open

Feat(validation): Add support for Prefix keys#93
ClausHolbechArista wants to merge 24 commits into
aristanetworks:mainfrom
ClausHolbechArista:prefix-keys

Conversation

@ClausHolbechArista

@ClausHolbechArista ClausHolbechArista commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

Change Summary

Add support for Prefix keys like custom_structured_configuration_

Also needs a follow up PR updating the metaschema and schema in AVD.

Related Issue(s)

Get proper validation of custom structured configuration.
Fixes #77 since it is actually caused by missing validation of custom structured configuration.

Component(s) name

Proposed changes

  • Add support for prefix_keys to the dict schema.
    • Example for custom_structured_configuration_ in YAML schema format:
      type: dict
      keys:
        custom_structured_configuration_prefix:
          type: list
          items:
            type: str
          default: ["custom_structured_configuration_"]
      dynamic_keys:
        <...>
      prefix_keys: # Using prefixes derived from user input"
        - prefixes_key: "custom_structured_configuration_prefixes"
          include_suffix_in_data: true
          schema_ref: "eos_cli_config_gen#"
    • Similar example that could be added to avoid dynamic keys:
      type: dict
      keys:
        <...>
      dynamic_keys:
        <...>
      prefix_keys: # Using static prefix
        - prefixes: ["network_services_"]
          include_suffix_in_data: false
          schema_ref: "eos_designs#/$defs/network_services"

How to test

Added unit tests here. Will need an AVD change to fully test it end to end.

Checklist

User Checklist

  • N/A

Repository Checklist

  • My code has been rebased from main before I start
  • I have read the CONTRIBUTING document.
  • I have updated testing accordingly. (check the box if not applicable)

Summary by CodeRabbit

  • New Features

    • Added support for prefix-based dynamic key matching in dictionary schemas with validation enhancements.
  • Bug Fixes & Improvements

    • Enhanced dictionary key validation with improved error handling and coercion behavior.
  • Dependencies

    • Updated Rust dependencies including pyo3, serde_json, and cipher libraries.
    • Updated GitHub Actions and development tools to pinned, secure versions.
  • Chores

    • Version bumped to 0.0.6.
    • Added automated code quality checks and security auditing workflows.

@github-actions

Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions

Copy link
Copy Markdown

Conflicts have been resolved. A maintainer will review the pull request shortly.

@codecov

codecov Bot commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.16834% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.20%. Comparing base (3c92c5a) to head (0b90d72).
⚠️ Report is 38 commits behind head on main.

Files with missing lines Patch % Lines
rust/avdschema/src/utils/schema_from_path.rs 80.53% 22 Missing ⚠️
rust/yaml-parser/src/avdschema_data_impl.rs 52.17% 11 Missing ⚠️
rust/avdschema/src/schema/dict/mod.rs 98.72% 8 Missing ⚠️
rust/avdschema/src/schema/any.rs 0.00% 5 Missing ⚠️
rust/avdschema/src/schema/dict/prefix_keys.rs 98.35% 4 Missing ⚠️
rust/pyvalidation/src/lib.rs 84.61% 2 Missing ⚠️
rust/yaml-parser/src/lexer/quoted.rs 85.71% 2 Missing ⚠️
rust/avdschema/src/utils/load.rs 0.00% 1 Missing ⚠️
rust/pypasswords/src/lib.rs 90.90% 1 Missing ⚠️
rust/validation/src/feedback.rs 97.56% 1 Missing ⚠️
... and 4 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #93      +/-   ##
==========================================
+ Coverage   88.63%   89.20%   +0.56%     
==========================================
  Files          79       81       +2     
  Lines       15304    16364    +1060     
==========================================
+ Hits        13565    14597    +1032     
- Misses       1739     1767      +28     
Components Coverage Δ
Python 100.00% <100.00%> (ø)
Rust 89.19% <96.16%> (+0.56%) ⬆️
Files with missing lines Coverage Δ
pyavd_utils/__init__.py 100.00% <100.00%> (ø)
rust/avdschema/src/inherit.rs 96.02% <ø> (ø)
rust/avdschema/src/resolve/mod.rs 91.46% <100.00%> (ø)
rust/avdschema/src/resolve/resolve_ref.rs 100.00% <ø> (ø)
rust/avdschema/src/resolve/walker.rs 77.55% <100.00%> (ø)
rust/avdschema/src/schema/dict/dynamic_keys.rs 100.00% <100.00%> (ø)
rust/avdschema/src/schema/list.rs 100.00% <ø> (ø)
rust/avdschema/src/schema/str.rs 100.00% <100.00%> (ø)
rust/avdschema/src/store.rs 70.00% <100.00%> (ø)
rust/avdschema/src/utils/dump.rs 86.95% <100.00%> (ø)
... and 30 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pre-commit-ci Bot and others added 2 commits May 26, 2026 15:48
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

ClausHolbechArista and others added 14 commits June 1, 2026 15:06
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
…p across 1 directory (aristanetworks#143)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…12e35e16 to e79a6962e0d4c0c17b229090214935d2e33f8354 (aristanetworks#147)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…0dab7590f9b7d to cef221092ed1bacb1cc03d23a2d87d1d172e277b (aristanetworks#145)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…istanetworks#146)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
dependabot Bot and others added 6 commits June 3, 2026 17:24
…rks#150)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR upgrades pyavd-utils from v0.0.5 to v0.0.6, introducing a major refactor of dictionary schema key resolution with support for dynamic-key overrides and prefix-based matching, while hardening CI workflows with pinned action versions and explicit permissions. The changes span schema definitions, validation pipelines, Python bindings, dependency updates, and tooling configuration.

Changes

Dictionary Schema Refactor with Prefix and Dynamic Key Support

Layer / File(s) Summary
Dictionary key resolution contract and types
rust/avdschema/src/schema/dict/dynamic_keys.rs
Defines DynamicKeyOverrides, DefaultDynamicKeys, DynamicKeyInfo, DictKeyMatch enum, and ResolvedDictKeys struct representing the unified key-matching contract.
Dynamic key resolution and caching
rust/avdschema/src/schema/dict/mod.rs
Implements Dict struct with dynamic-key extraction from input/defaults, dot-path traversal, override precedence handling, and lazy caching of computed keys.
Prefix-based key matching and suffix resolution
rust/avdschema/src/schema/dict/prefix_keys.rs
Adds PrefixKeys configuration, prefix resolution logic with suffix-key schema matching, and Dict APIs for prefix-key lookup and validation.
Remove old dict.rs and re-export new types
rust/avdschema/src/lib.rs
Deletes monolithic dict.rs and re-exports DynamicKeyOverrides from the root crate for public API consumption.

Schema Utilities Updated for Key Resolution

Layer / File(s) Summary
SchemaDataMapping trait extended for key iteration
rust/avdschema/src/utils/schema_data.rs
Adds Keys associated iterator type and keys() method to trait; updates serde_json and YAML implementations to yield string keys as Cow<'a, str>.
Schema path resolution supporting dynamic overrides and prefix keys
rust/avdschema/src/utils/schema_from_path.rs
Refactors SchemaKey enum with renamed/new variants, updates path-lookup functions to accept Store and dynamic_key_overrides, implements DictKeyMatch-based resolution logic.
Restrict internal utility module visibility
rust/avdschema/src/utils/mod.rs
Makes dump, load, macros, schema_data, and schema_from_path modules crate-restricted instead of public.

Validation Dict Key Matching with Dynamic Override Support

Layer / File(s) Summary
Dict validation refactored for unified key matching
rust/validation/src/validation/dict.rs
Rewrites validate_keys to resolve all keys via schema.resolve_dict_keys(...), dispatches on DictKeyMatch variants, centralizes deprecation checking, and preserves EOS Config warnings.
Dict validation tests including prefix and override scenarios
rust/validation/src/validation/dict.rs (tests)
Adds imports for Arc, DynamicKeyOverrides, StoreValidate, and extensive test cases for dynamic-key overrides and large suite of prefix-key validation scenarios.
Configuration context extended for dynamic overrides
rust/validation/src/context.rs
Adds dynamic_key_overrides: Option<Arc<DynamicKeyOverrides>> field to Configuration struct for passing overrides through validation pipelines.
Supporting validation refactors and pattern updates
rust/validation/src/validation/*, rust/validation/src/validatable/*
Updates modules to use `then(
YAML parser iterator and schema data updates
rust/yaml-parser/src/avdschema_data_impl.rs
Adds YamlMappingKeys iterator for SchemaDataMapping, updates test callsites for new function signatures accepting Store and dynamic-key overrides.

AVD Schema Crate Lint and Visibility Updates

Layer / File(s) Summary
Crate-level lint inheritance and visibility adjustments
rust/avdschema/src/lib.rs, rust/avdschema/src/resolve/mod.rs, rust/validation/src/lib.rs, rust/passwords/src/lib.rs
Adds #![allow(...)] lint suppressions with explicit reasons; restricts resolve and utils module visibility to pub(crate).
Schema crate code pattern updates
rust/avdschema/src/**/*.rs, rust/validation/src/**/*.rs
Adjusts trait imports to use as _, replaces to_string() with to_owned()/clone(), updates documentation comments, and refactors conditionals.

Password Codec Dependency and Binding Updates

Layer / File(s) Summary
CBC encryption/decryption with updated trait methods
rust/passwords/src/cbc/mod.rs, rust/passwords/Cargo.toml
Switches to BlockModeEncrypt/BlockModeDecrypt traits from cbc::cipher and updates encryption/decryption to use non-_mut method variants.
Python binding signatures refactored to use borrowed strings
rust/pypasswords/src/lib.rs
Changes sha512_crypt, cbc_*, and simple_7_* functions from pub fn with owned String to pub(crate) fn with &str parameters, adjusting error mapping accordingly.
Password codec test and config updates
rust/passwords/src/, rust/pypasswords/src/tests/, rust/pyvalidation/
Adds lint allowlist, updates simple_7 test formatting, adjusts Criterion benchmarks, and refactors PyO3 build script.

Version Bump and Dependency Updates (v0.0.5 → v0.0.6)

Layer / File(s) Summary
Version number updates across manifest files
Cargo.toml, pyavd_utils/__init__.py, pyproject.toml
Updates workspace version, package versions, and bumpversion configuration from 0.0.5 to 0.0.6.
Workspace and dev dependency version updates
Cargo.toml, rust/*/Cargo.toml
Bumps pyo3-build-config, pyo3-log, serde_json, serde_with, and crypto crates (cbc, cipher, des, base64); updates coverage[toml] and criterion dev dependencies.
Third-party license file updated for new versions
pyavd_utils/THIRD_PARTY_LICENSES.txt
Updates license metadata and copyright years for all bumped crates; adds new entry for hybrid-array 0.4.12.

GitHub Actions Workflows: Permissions and Action Pinning

Layer / File(s) Summary
Add explicit permissions blocks to workflows
.github/workflows/*.yml
Adds workflow-level permissions: contents: read and job-level permissions (e.g., id-token: write for OIDC publishing) to build-wheel, codecov, pull-request-management, rn-labeler, triage, release, and stale workflows.
Pin GitHub Actions to specific commit SHAs
.github/workflows/*.yml
Updates actions/checkout, actions/setup-python, actions/upload/download-artifact, codecov/codecov-action, and third-party actions (auto-author-assign, action-semantic-pull-request, actions-label-merge-conflict, actions/stale) to specific commit hashes.
Workflow feature updates and configuration changes
.github/workflows/build-wheel.yml, .github/actions/rn-pr-labeler-action/action.yml, .github/workflows/pull-request-autofix.yml, .github/workflows/zizmor.yml
Consolidates rn-labeler action into single bash script, adds pre-commit/rustfmt/autofix CI workflow, introduces new zizmor audit workflow, and uses environment variables for version bump configuration.
Remove pre-commit lint steps from PR management workflow
.github/workflows/pull-request-management.yml
Deletes prior pre-commit/rustfmt/cargo linting block from cargo_build_and_test job, consolidating linting into dedicated autofix workflow.

Pre-Commit and Dependabot Configuration Updates

Layer / File(s) Summary
Pre-commit hooks migrated to arista-netdevops-community repo
.pre-commit-config.yaml
Replaces Rust hooks from doublify/pre-commit-rust and EmbarkStudios/cargo-deny with equivalent hooks from arista-netdevops-community/pre-commit-hooks, including updated args and file filters.
Add zizmor audit and update tool versions
.pre-commit-config.yaml
Adds new zizmor-pre-commit hook for .github/ audits, bumps ruff-pre-commit revision, and increments pyright-python patch version.
Dependabot cooldown configuration
.github/dependabot.yml
Adds cooldown block with default-days: 7 to pip, github-actions, cargo, and rust-toolchain Dependabot entries to space out automated update PRs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Possibly related PRs

  • aristanetworks/pyavd-utils#133: Extends dict key resolution by wiring DynamicKeyOverrides through the same schema-key lookup and validation codepaths.
  • aristanetwork/pyavd-utils#140: Consolidates the rn-pr-labeler-action composite step into a single bash script, matching this PR's action rewrite.
  • aristanetworks/pyavd-utils#132: Adds pull-request-autofix.yml workflow and reconfigures pre-commit hooks to use the arista-netdevops-community repo, overlapping at the same config level.

Suggested reviewers

  • gmuloc

🐰 A dict with prefixes so fine,
Dynamic keys in perfect line,
Overrides flow through the stack,
Version bumped—we're on track!
With zizmor watching, all is right,
CI pinned, our workflows tight. 🔒

@github-actions

Copy link
Copy Markdown

Conflicts have been resolved. A maintainer will review the pull request shortly.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/pull-request-autofix.yml:
- Around line 14-15: The workflow currently sets repository contents permission
to read (permissions: contents: read) which prevents the autofix action from
pushing commits; change the permissions block to grant write access for contents
(so the autofix-ci/action can create commits) by updating the permissions key
that references contents: read to allow write (e.g., contents: write) and ensure
the job that runs autofix-ci/action retains that permission so the push step
succeeds.

In `@Cargo.toml`:
- Around line 13-15: The THIRD_PARTY_LICENSES.txt metadata is out of sync with
Cargo.toml/Cargo.lock: Cargo.toml pins pyo3-build-config at 0.28.3 but the
license file (pyavd_utils/THIRD_PARTY_LICENSES.txt) still lists
pyo3-build-config (and other PyO3 entries) as 0.27.2; update the license
metadata to match the actual dependency version (0.28.3) by regenerating the
license file using your license-export tool or run the same process that
produced THIRD_PARTY_LICENSES.txt, or alternatively align the Cargo.toml pins
back to 0.27.2 so versions match consistently (referencing pyo3-build-config,
pyo3, pyo3-log entries when making the change).

In `@rust/avdschema/src/schema/dict/dynamic_keys.rs`:
- Around line 62-80: The current search over self.prefix_configs uses find_map
which returns the first non-None result and can let an early
InvalidSuffix/AllowedOtherSuffix mask a later Valid match; change the logic in
the block that calls prefix_configs.iter().find_map(...) so that you iterate all
configs: return immediately if any config.resolve_match(key) yields
PrefixMatchResult::Valid (wrap into DictKeyMatch::Prefix), otherwise remember
the first seen InvalidSuffix or AllowedOtherSuffix and after the loop return
that remembered DictKeyMatch variant if present, else DictKeyMatch::UnknownKey;
reference symbols: prefix_configs, resolve_match,
PrefixMatchResult::{Valid,InvalidSuffix,AllowedOtherSuffix}, and
DictKeyMatch::{Prefix,PrefixInvalidSuffix,PrefixAllowedOtherSuffix,UnknownKey}.

In `@rust/pyvalidation/benches/performance.rs`:
- Around line 19-27: The benchmark currently calls init_store_from_file inside
bencher.iter which only does real initialization on the first call (OnceLock),
so subsequent iterations measure the OnceLock error path; fix by not calling
init_store_from_file repeatedly: perform a single initialization outside the
bencher.iter (call init_store_from_file(schema_file.to_owned()) once before
group.bench_function) if you only need one-time setup, or if you actually want
to measure cold initialization cost, change the benchmark to spawn a fresh
process per iteration (or use a per-iteration external setup) so
init_store_from_file runs in a new process each time; refer to
init_store_from_file, OnceLock, and the group.bench_function/bencher.iter call
to locate where to change.

In `@rust/validation/src/lib.rs`:
- Around line 4-25: Replace the large crate-level allow block with a documented,
tracked remediation plan: open a tracking issue (link/ID) for removing/triaging
the lint suppressions and update the file's allow attribute (#![allow(...)] in
lib.rs) to either narrow scope (apply to specific test modules or files) or keep
only unavoidable lints while leaving a TODO comment referencing the new issue;
prioritize fixing clippy::indexing_slicing and missing_docs first and list them
in the issue with incremental tasks to remove each lint suppression from the
allow list (mention the exact lint names from the current block such as
missing_docs and clippy::indexing_slicing and any other high-risk entries).
🪄 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: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b6e6681c-0007-434e-a5ce-b1020d1013aa

📥 Commits

Reviewing files that changed from the base of the PR and between 2b11536 and 0b90d72.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (67)
  • .github/actions/rn-pr-labeler-action/action.yml
  • .github/dependabot.yml
  • .github/workflows/build-wheel.yml
  • .github/workflows/codecov.yml
  • .github/workflows/pull-request-autofix.yml
  • .github/workflows/pull-request-conflict.yml
  • .github/workflows/pull-request-management.yml
  • .github/workflows/pull-request-rn-labeler.yml
  • .github/workflows/pull-request-triage.yml
  • .github/workflows/release.yml
  • .github/workflows/stale.yml
  • .github/workflows/zizmor.yml
  • .pre-commit-config.yaml
  • Cargo.toml
  • pyavd_utils/THIRD_PARTY_LICENSES.txt
  • pyavd_utils/__init__.py
  • pyproject.toml
  • rust/avdschema/Cargo.toml
  • rust/avdschema/src/inherit.rs
  • rust/avdschema/src/lib.rs
  • rust/avdschema/src/resolve/mod.rs
  • rust/avdschema/src/resolve/resolve_ref.rs
  • rust/avdschema/src/resolve/walker.rs
  • rust/avdschema/src/schema/any.rs
  • rust/avdschema/src/schema/base/documentation_options.rs
  • rust/avdschema/src/schema/base/mod.rs
  • rust/avdschema/src/schema/dict.rs
  • rust/avdschema/src/schema/dict/dynamic_keys.rs
  • rust/avdschema/src/schema/dict/mod.rs
  • rust/avdschema/src/schema/dict/prefix_keys.rs
  • rust/avdschema/src/schema/list.rs
  • rust/avdschema/src/schema/str.rs
  • rust/avdschema/src/store.rs
  • rust/avdschema/src/utils/dump.rs
  • rust/avdschema/src/utils/load.rs
  • rust/avdschema/src/utils/macros.rs
  • rust/avdschema/src/utils/mod.rs
  • rust/avdschema/src/utils/schema_data.rs
  • rust/avdschema/src/utils/schema_from_path.rs
  • rust/passwords/Cargo.toml
  • rust/passwords/src/cbc/mod.rs
  • rust/passwords/src/lib.rs
  • rust/passwords/src/simple_7/mod.rs
  • rust/pypasswords/Cargo.toml
  • rust/pypasswords/build.rs
  • rust/pypasswords/src/lib.rs
  • rust/pypasswords/src/tests/mod.rs
  • rust/pypasswords/src/tests/test_simple_7.rs
  • rust/pyvalidation/Cargo.toml
  • rust/pyvalidation/benches/performance.rs
  • rust/pyvalidation/build.rs
  • rust/pyvalidation/src/lib.rs
  • rust/validation/Cargo.toml
  • rust/validation/src/context.rs
  • rust/validation/src/feedback.rs
  • rust/validation/src/lib.rs
  • rust/validation/src/validatable/mod.rs
  • rust/validation/src/validatable/serde_json_impl.rs
  • rust/validation/src/validatable/tests.rs
  • rust/validation/src/validatable/yaml_parser_impl.rs
  • rust/validation/src/validation/boolean.rs
  • rust/validation/src/validation/dict.rs
  • rust/validation/src/validation/int.rs
  • rust/validation/src/validation/list.rs
  • rust/validation/src/validation/store.rs
  • rust/validation/src/validation/str.rs
  • rust/yaml-parser/src/avdschema_data_impl.rs
💤 Files with no reviewable changes (1)
  • rust/avdschema/src/schema/dict.rs

Comment on lines +14 to +15
permissions:
contents: read

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Grant write permission for autofix commits.

Line 14-15 sets contents: read, but Line 47-48 attempts to push fixes via autofix-ci/action. With read-only contents, the commit-back step cannot write.

Suggested fix
 permissions:
-  contents: read
+  contents: write
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
permissions:
contents: read
permissions:
contents: write
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/pull-request-autofix.yml around lines 14 - 15, The
workflow currently sets repository contents permission to read (permissions:
contents: read) which prevents the autofix action from pushing commits; change
the permissions block to grant write access for contents (so the
autofix-ci/action can create commits) by updating the permissions key that
references contents: read to allow write (e.g., contents: write) and ensure the
job that runs autofix-ci/action retains that permission so the push step
succeeds.

Comment thread Cargo.toml
Comment on lines 13 to +15
pyo3 = { version = "0.27.2" }
pyo3-build-config = { version = "0.27.2" }
pyo3-log = { version = "0.13.1" }
pyo3-build-config = { version = "0.28.3" }
pyo3-log = { version = "0.13.3" }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Workspace pins in Cargo.toml =="
rg -n '^\s*(pyo3|pyo3-build-config|pyo3-log)\s*=' Cargo.toml

echo
echo "== Resolved versions in Cargo.lock =="
LOCK_FILE="$(fd '^Cargo.lock$' | head -n1)"
if [ -n "${LOCK_FILE:-}" ]; then
  rg -n '^(name = "(pyo3|pyo3-build-config|pyo3-log)"|version = )' "$LOCK_FILE" -A1
else
  echo "Cargo.lock not found"
fi

echo
echo "== License manifest entries =="
rg -n 'pyo3-build-config|pyo3-ffi|pyo3-macros-backend|pyo3-macros|pyo3 0\.27\.2' pyavd_utils/THIRD_PARTY_LICENSES.txt

Repository: aristanetworks/pyavd-utils

Length of output: 27960


Align PyO3 dependency versions with generated license metadata (pyo3-build-config 0.28.3).

Cargo.toml (lines 13-15) pins pyo3-build-config to 0.28.3, Cargo.lock includes pyo3-build-config 0.28.3, but pyavd_utils/THIRD_PARTY_LICENSES.txt still lists pyo3-build-config 0.27.2 (with the rest of the PyO3 family at 0.27.2). This is a dependency/metadata drift that can break compliance/release documentation consistency—regenerate/update the license metadata to include 0.28.3 (or align the dependency pins).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Cargo.toml` around lines 13 - 15, The THIRD_PARTY_LICENSES.txt metadata is
out of sync with Cargo.toml/Cargo.lock: Cargo.toml pins pyo3-build-config at
0.28.3 but the license file (pyavd_utils/THIRD_PARTY_LICENSES.txt) still lists
pyo3-build-config (and other PyO3 entries) as 0.27.2; update the license
metadata to match the actual dependency version (0.28.3) by regenerating the
license file using your license-export tool or run the same process that
produced THIRD_PARTY_LICENSES.txt, or alternatively align the Cargo.toml pins
back to 0.27.2 so versions match consistently (referencing pyo3-build-config,
pyo3, pyo3-log entries when making the change).

Comment on lines +62 to +80
self.prefix_configs
.as_ref()
.and_then(|prefix_configs| {
prefix_configs
.iter()
.find_map(|config| match config.resolve_match(key) {
Some(PrefixMatchResult::Valid(prefix_key_match)) => {
Some(DictKeyMatch::Prefix(prefix_key_match))
}
Some(PrefixMatchResult::InvalidSuffix) => {
Some(DictKeyMatch::PrefixInvalidSuffix)
}
Some(PrefixMatchResult::AllowedOtherSuffix) => {
Some(DictKeyMatch::PrefixAllowedOtherSuffix)
}
None => None,
})
})
.unwrap_or(DictKeyMatch::UnknownKey)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Prefer valid prefix matches over early invalid-prefix outcomes across configs.

The current find_map returns the first non-None prefix result, so InvalidSuffix/AllowedOtherSuffix from an earlier config can mask a valid match from a later config.

Suggested fix
-        self.prefix_configs
-            .as_ref()
-            .and_then(|prefix_configs| {
-                prefix_configs
-                    .iter()
-                    .find_map(|config| match config.resolve_match(key) {
-                        Some(PrefixMatchResult::Valid(prefix_key_match)) => {
-                            Some(DictKeyMatch::Prefix(prefix_key_match))
-                        }
-                        Some(PrefixMatchResult::InvalidSuffix) => {
-                            Some(DictKeyMatch::PrefixInvalidSuffix)
-                        }
-                        Some(PrefixMatchResult::AllowedOtherSuffix) => {
-                            Some(DictKeyMatch::PrefixAllowedOtherSuffix)
-                        }
-                        None => None,
-                    })
-            })
-            .unwrap_or(DictKeyMatch::UnknownKey)
+        if let Some(prefix_configs) = self.prefix_configs.as_ref() {
+            let mut fallback: Option<DictKeyMatch<'a>> = None;
+            for config in prefix_configs {
+                match config.resolve_match(key) {
+                    Some(PrefixMatchResult::Valid(prefix_key_match)) => {
+                        return DictKeyMatch::Prefix(prefix_key_match);
+                    }
+                    Some(PrefixMatchResult::AllowedOtherSuffix) => {
+                        fallback.get_or_insert(DictKeyMatch::PrefixAllowedOtherSuffix);
+                    }
+                    Some(PrefixMatchResult::InvalidSuffix) => {
+                        if fallback.is_none() {
+                            fallback = Some(DictKeyMatch::PrefixInvalidSuffix);
+                        }
+                    }
+                    None => {}
+                }
+            }
+            return fallback.unwrap_or(DictKeyMatch::UnknownKey);
+        }
+        DictKeyMatch::UnknownKey
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/avdschema/src/schema/dict/dynamic_keys.rs` around lines 62 - 80, The
current search over self.prefix_configs uses find_map which returns the first
non-None result and can let an early InvalidSuffix/AllowedOtherSuffix mask a
later Valid match; change the logic in the block that calls
prefix_configs.iter().find_map(...) so that you iterate all configs: return
immediately if any config.resolve_match(key) yields PrefixMatchResult::Valid
(wrap into DictKeyMatch::Prefix), otherwise remember the first seen
InvalidSuffix or AllowedOtherSuffix and after the loop return that remembered
DictKeyMatch variant if present, else DictKeyMatch::UnknownKey; reference
symbols: prefix_configs, resolve_match,
PrefixMatchResult::{Valid,InvalidSuffix,AllowedOtherSuffix}, and
DictKeyMatch::{Prefix,PrefixInvalidSuffix,PrefixAllowedOtherSuffix,UnknownKey}.

Comment on lines +19 to 27
fn benchmark_init_store_from_file(criterion: &mut Criterion) {
let schema_file = get_store_gz_path();
let mut group = c.benchmark_group("sample-size-10");
let mut group = criterion.benchmark_group("sample-size-10");
group.sample_size(10); // Lowering the sample size from the default 1000 since tests in this group are expected to take longer.
group.bench_function("init_store_from_fragments", |b| {
b.iter(|| init_store_from_file(schema_file.to_owned()))
group.bench_function("init_store_from_fragments", |bencher| {
bencher.iter(|| init_store_from_file(schema_file.to_owned()));
});
group.finish();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Benchmark is measuring the OnceLock error path after the first iteration.

init_store_from_file initializes a global OnceLock only once (rust/pyvalidation/src/lib.rs), so repeated calls inside bencher.iter mostly benchmark immediate failure, not store initialization/resolve cost.

Suggested fix
 use criterion::Criterion;
 use criterion::criterion_group;
 use criterion::criterion_main;
+use avdschema::Store;
 use pyvalidation::validation::get_validated_data;
-use pyvalidation::validation::init_store_from_file;
 use test_schema_store::get_store_gz_path;
@@
 fn benchmark_init_store_from_file(criterion: &mut Criterion) {
     let schema_file = get_store_gz_path();
     let mut group = criterion.benchmark_group("sample-size-10");
     group.sample_size(10); // Lowering the sample size from the default 1000 since tests in this group are expected to take longer.
     group.bench_function("init_store_from_fragments", |bencher| {
-        bencher.iter(|| init_store_from_file(schema_file.to_owned()));
+        bencher.iter(|| {
+            let store = Store::from_file(Some(&schema_file)).expect("load store");
+            let _ = store.as_resolved().expect("resolve store");
+        });
     });
     group.finish();
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn benchmark_init_store_from_file(criterion: &mut Criterion) {
let schema_file = get_store_gz_path();
let mut group = c.benchmark_group("sample-size-10");
let mut group = criterion.benchmark_group("sample-size-10");
group.sample_size(10); // Lowering the sample size from the default 1000 since tests in this group are expected to take longer.
group.bench_function("init_store_from_fragments", |b| {
b.iter(|| init_store_from_file(schema_file.to_owned()))
group.bench_function("init_store_from_fragments", |bencher| {
bencher.iter(|| init_store_from_file(schema_file.to_owned()));
});
group.finish();
}
use criterion::Criterion;
use criterion::criterion_group;
use criterion::criterion_main;
use avdschema::Store;
use pyvalidation::validation::get_validated_data;
use test_schema_store::get_store_gz_path;
fn benchmark_init_store_from_file(criterion: &mut Criterion) {
let schema_file = get_store_gz_path();
let mut group = criterion.benchmark_group("sample-size-10");
group.sample_size(10); // Lowering the sample size from the default 1000 since tests in this group are expected to take longer.
group.bench_function("init_store_from_fragments", |bencher| {
bencher.iter(|| {
let store = Store::from_file(Some(&schema_file)).expect("load store");
let _ = store.as_resolved().expect("resolve store");
});
});
group.finish();
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/pyvalidation/benches/performance.rs` around lines 19 - 27, The benchmark
currently calls init_store_from_file inside bencher.iter which only does real
initialization on the first call (OnceLock), so subsequent iterations measure
the OnceLock error path; fix by not calling init_store_from_file repeatedly:
perform a single initialization outside the bencher.iter (call
init_store_from_file(schema_file.to_owned()) once before group.bench_function)
if you only need one-time setup, or if you actually want to measure cold
initialization cost, change the benchmark to spawn a fresh process per iteration
(or use a per-iteration external setup) so init_store_from_file runs in a new
process each time; refer to init_store_from_file, OnceLock, and the
group.bench_function/bencher.iter call to locate where to change.

Comment on lines +4 to +25
// TODO: Reevaluate the allow
#![allow(
missing_docs,
missing_debug_implementations,
clippy::as_conversions,
clippy::cast_possible_truncation,
clippy::cast_precision_loss,
clippy::default_trait_access,
clippy::empty_structs_with_brackets,
clippy::field_scoped_visibility_modifiers,
clippy::float_cmp,
clippy::from_iter_instead_of_collect,
clippy::indexing_slicing,
clippy::min_ident_chars,
clippy::match_wildcard_for_single_variants,
clippy::option_option,
clippy::partial_pub_fields,
clippy::shadow_unrelated,
clippy::struct_excessive_bools,
clippy::trivially_copy_pass_by_ref,
reason = "Existing validation models and tests predate workspace lint inheritance"
)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Document technical debt: Large lint allowance block.

The broad #![allow(...)] block suppresses many important lints including missing_docs, clippy::indexing_slicing, and others. While the TODO acknowledges this needs re-evaluation, consider tracking this technical debt with an issue to incrementally address these warnings.

Consider creating a tracking issue and incrementally addressing the most valuable lints first (e.g., clippy::indexing_slicing for safety, missing_docs for API clarity).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/validation/src/lib.rs` around lines 4 - 25, Replace the large
crate-level allow block with a documented, tracked remediation plan: open a
tracking issue (link/ID) for removing/triaging the lint suppressions and update
the file's allow attribute (#![allow(...)] in lib.rs) to either narrow scope
(apply to specific test modules or files) or keep only unavoidable lints while
leaving a TODO comment referencing the new issue; prioritize fixing
clippy::indexing_slicing and missing_docs first and list them in the issue with
incremental tasks to remove each lint suppression from the allow list (mention
the exact lint names from the current block such as missing_docs and
clippy::indexing_slicing and any other high-risk entries).

@github-actions

Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feat: Detect duplicate primary keys in validation in dict

2 participants