Skip to content

Conversation

@scunningham
Copy link
Contributor

No description provided.

@scunningham scunningham requested review from Copilot and tonymeehan May 28, 2025 19:20
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the HashRule function to ignore non-semantic metadata (version, generation counter, and existing hash) when computing the rule’s hash, ensuring a stable identifier across metadata-only changes.

  • Changed HashRule signature to accept a ParseRuleT and removed the old any version.
  • Zeroes out Metadata.Gen, Metadata.Version, and Metadata.Hash before marshaling.
  • Marshals the cleaned rule object for deterministic hashing.
Comments suppressed due to low confidence (2)

pkg/parser/tree.go:709

  • Consider adding unit tests to verify that changes to Gen, Version, and Hash metadata do not affect the computed rule hash.
rule.Metadata.Gen = 0      // Gen is bumped on every semantic change, so we don't want it in the hash

pkg/parser/tree.go:707

  • Modifying the rule parameter in-place may lead to unintended side-effects if ParseRuleT contains reference types; consider deep copying or documenting this assumption.
// NOTE: Modify 'rule' is acceptable because parameter passed by value.

@scunningham scunningham merged commit 923e9a1 into prequel-dev:main May 28, 2025
1 check passed
@scunningham scunningham deleted the stricthash branch May 28, 2025 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants