Moves away from strict Violation enum to open Violation based on Advice

[jsuereth]

Changes

• Remove Advice and SemconvAttribute as types, only use PolicyFinding now.
• Hide sub-mod in weaver_checker crate, so everything is top-level now.
• Create a custom deserializer to support existing Rego policies for both Advice and SemconvAttribute in PolicyFinding
• Update tests to use Violation
• Update weaver_live_check to use Violation instead of Advice.
• Update docs

This should give us all the same goodness as live check going forward, in terms of flexible policies.

[codecov]

Codecov Report

❌ Patch coverage is 77.93103% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.6%. Comparing base (34a3e13) to head (4308ad5).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/weaver_checker/src/finding.rs	76.1%	20 Missing ⚠️
crates/weaver_live_check/src/advice.rs	72.5%	11 Missing ⚠️
crates/weaver_live_check/src/lib.rs	87.5%	1 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #1038   +/-   ##
=====================================
  Coverage   78.6%   78.6%           
=====================================
  Files         82      82           
  Lines       6701    6772   +71     
=====================================
+ Hits        5268    5329   +61     
- Misses      1433    1443   +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jerbly]

I guess it doesn't matter too much as it's internal, but I'm curious what was behind the decision to rename Advice to Violation, rather than Violation to Advice? Since now we can have Violations that are not Violations whereas, before one type of Advice was a Violation.

[jsuereth]

I guess it doesn't matter too much as it's internal, but I'm curious what was behind the decision to rename Advice to Violation, rather than Violation to Advice? Since now we can have Violations that are not Violations whereas, before one type of Advice was a Violation.

@jerbly I actually don't like the name Violation or Advice here. I think Advice is a bit too tied to live-check (and doesn't have any connotation it can be used to fail things), and Violation makes it awkward to make informational/warning messages.

I asked gemini and it suggested Report or PolicyFinding here, where inside Report you can have informational/advice, warning and violation. WDYT of these names?

[lmolkova]

Nice!

Could you please update docs:

• https://github.com/jsuereth/weaver/blob/1bfbf330e04d258d31b13ab9017374ac092d1937/crates/weaver_live_check/README.md#advice and the changelog
• https://github.com/jsuereth/weaver/blob/1bfbf330e04d258d31b13ab9017374ac092d1937/crates/weaver_checker/README.md#policy-examples
  ?

(it can be a follow up)

and add a changelog record

[lquerel]

I asked gemini and it suggested Report or PolicyFinding here, where inside Report you can have informational/advice, warning and violation. WDYT of these names?

@jsuereth personally I prefer PolicyFinding because it's more informative.

[lquerel]

It's really great to have this unification of results between weaver registry check and weaver registry live-check. As I mentioned in another comment, I have a clear preference for PolicyFinding instead of Violation or Report.

[jsuereth]

I'll updated to PolicyFinding before merging

[jsuereth]

@jerbly Given this is a breaking change, I'd love approval from all maintainers on this one. PTAL again - I started updating docs for live-check and realized this impacts it a bit more heavily and we may want to rename portions of the report output.

Let me know how you feel about that and whether or not it's ok / the time to do this.

[jerbly]

PolicyFinding makes sense.

[jsuereth]

Thanks all! Going to merge this for the next release.