Validate strict weslaw bindings#534
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds strict weslaw binding and semantic canonical hashing, contract-bundle manifest and law-diff artifacts, CLI ChangesLaw binding, hashing, and contract bundle manifest
Sequence Diagrams sequenceDiagram
participant CLI as Wesley CLI
participant Core as wesley-core
participant Emitter as wesley-emit-rust
participant FS as Filesystem
CLI->>Core: load schema + law, lower to IR, validate bindings, build contract-bundle manifest
Core-->>CLI: manifest + hashes (schema/law/document/profile/bundle)
CLI->>Emitter: emit request with manifest and IR
Emitter->>FS: write generated Rust with WESLEY_SCHEMA_HASH / WESLAW_HASH and validators
CLI->>FS: write metadata sidecar with manifest and law_ir_codec
Estimated code review effort: Suggested labels:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 The Case of Pull Request #534Plain-English Readout
Suggested next actions
📚 Glossary (what the Holmes terms mean)
🕵️ SHA-lock HOLMES full report (click to expand)🕵️ SHA-lock HOLMES Investigation
🔍 Executive Deduction"Watson, after careful examination of the evidence, I deduce..." Weighted Completion: ██████████ 95.0% 🧩 SCS Breakdown
🧪 TCI Breakdown
|
| Component | Risk Share | Points | Count |
|---|---|---|---|
| Drops | 0.0% | 0 | 0 |
| Renames Without Uid | 0.0% | 0 | 0 |
| Add Not Null Without Default | 100.0% | 1 | 1 |
| Non Concurrent Indexes | 0.0% | 0 | 0 |
📊 The Weight of Evidence
"Observe, Watson, how not all features carry equal importance..."
| Element | Weight | Status | Evidence | Strength | Deduction |
|---|---|---|---|---|---|
| schema | 5 | ✅ Exact SQL & tests | test/fixtures/examples/.wesley-cache/shipme-fixture/tests.sql:1-1@e35ed41 | exact | Elementary! |
🚪 Security & Performance Gates
"Elementary security measures, Watson..."
| Gate | Status | Evidence | Holmes's Ruling |
|---|---|---|---|
| Migration Risk | ✅ | MRI: 10.0% | "Trivial risk" |
| Test Coverage | ✅ | TCI: 90.0% | "Excellent coverage" |
| Sensitive Fields | ✅ | 0 fields | "All secured" |
| Evidence Quality | ✅ | 2 exact · 0 whole-file · 0 coarse | "All 2 citations resolve to exact line spans." |
📋 The Verdict
✅ ELEMENTARY - Ship immediately!
"The evidence is conclusive. No mysteries remain."
Signed and sealed,
- S. Holmes, Consulting Detective
[END OF INVESTIGATION FOR COMMIT e35ed41]
🧵 Command Run
- Run ID: run-fe7184b1-e909-4a09-a3e9-20dfbd9302dd
- Transmutation: holmes-investigate
- Command: investigate
- Status: completed
- Ledger: /home/runner/work/wesley/wesley/test/fixtures/examples/.wesley-cache/ledger
🩺 Dr. WATSON full report (click to expand)
🩺 Dr. Watson's Independent Verification Report
Medical Examination of Evidence
- Examination Date: 2026-05-26T02:44:26.061Z
- Patient SHA: e35ed41
🔬 Citation Verification
"Let me examine each piece of evidence independently..."
- Citations Examined: 2
- Verified: 0 ✅
- Failed: 0 ❌
- Unable to Verify: 2
- Exact Subrange Citations: 0
- Whole-file Citations: 0
- Coarse Citations: 0
- Evidence Trust: missing
- Trust Note: No evidence citations were available for trust analysis.
Verification Rate: 0.0%
📊 Mathematical Verification
"I shall recalculate Holmes's arithmetic..."
Holmes claimed SCS: 95.0%
Watson calculates: 100.0%
Difference:
🔍 Consistency Analysis
"Checking for contradictions in Holmes's deductions..."
✅ No logical inconsistencies detected
🩺 Dr. Watson's Medical Opinion
VERIFICATION: CONCERNS NOTED
"While Holmes's methods are generally sound, I have noted some"
"discrepancies that warrant further investigation. No evidence citations were available for trust analysis."
Respectfully submitted,
- Dr. J. Watson, M.D.
Medical Examiner & Verification Specialist
🧵 Command Run
- Run ID: run-d6e0c8f4-d2ca-4386-a193-fc37928ec4d9
- Transmutation: watson-verify
- Command: verify
- Status: completed
- Ledger: /home/runner/work/wesley/wesley/test/fixtures/examples/.wesley-cache/ledger
🔮 Professor MORIARTY full report (click to expand)
🧠 Professor Moriarty's Temporal Predictions
The Mathematics of Inevitability
- Analysis Date: 2026-05-26T02:45:08.194Z
INSUFFICIENT DATA
"I require at least two data points to predict the future."
"Run Wesley generate multiple times to build history."
🧵 Command Run
- Run ID: run-943af335-7b38-4834-9e06-a9194b42fda8
- Transmutation: moriarty-predict
- Command: predict
- Status: completed
- Ledger: /home/runner/work/wesley/wesley/test/fixtures/examples/.wesley-cache/ledger
Machine-readable reports: holmes-report.json · watson-report.json · moriarty-report.json (see workflow artifacts).
Filed at 221B Repository Street
![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: ef6ae4faf2
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
crates/wesley-emit-rust/src/lib.rs (1)
33-47: ⚡ Quick winAdd a direct unit test for hash-constant emission.
The new public API and print path are critical output contracts; a small crate-local test that asserts both constants are emitted (and parse) would harden against regressions.
Also applies to: 296-306
🤖 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 `@crates/wesley-emit-rust/src/lib.rs` around lines 33 - 47, Add a crate-local unit test that calls emit_rust_with_operations_and_hashes with a minimal WesleyIR and empty operations plus sample schema_hash and law_hash strings, then assert the returned String contains the emitted constants (look for the constant identifiers or the exact hash values) and verify the output is valid Rust by parsing it (e.g., with syn::parse_file or another Rust parser). Put the test in the same crate (e.g., a #[cfg(test)] mod tests with #[test] fn emit_hash_constants_present()), reference the public function emit_rust_with_operations_and_hashes and the RustProvenanceConstants identifiers to locate code, and fail the test if the constants are missing or the parse fails.crates/wesley-cli/tests/cli.rs (1)
773-776: ⚡ Quick winAssert
WESLEY_SCHEMA_HASHmatches metadata too.This test only enforces equality for
lawHash;schemaHashis currently checked for presence/prefix only, so a wrong emitted schema constant could slip through.Suggested test hardening
assert!(generated.contains("pub const WESLEY_SCHEMA_HASH: &'static str = \"sha256:")); assert!(generated.contains("pub const WESLAW_HASH: &'static str = \"sha256:")); + assert_eq!( + metadata_json["schemaHash"], + generated_hash_literal(&generated, "WESLEY_SCHEMA_HASH") + ); assert_eq!( metadata_json["lawHash"], generated_hash_literal(&generated, "WESLAW_HASH") );🤖 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 `@crates/wesley-cli/tests/cli.rs` around lines 773 - 776, The test currently asserts metadata_json["lawHash"] equals generated_hash_literal(&generated, "WESLAW_HASH") but does not assert the schema hash; update the test to also assert metadata_json["schemaHash"] equals generated_hash_literal(&generated, "WESLEY_SCHEMA_HASH") so the emitted schema constant is validated. Locate the assertion using metadata_json and generated_hash_literal in the test (around the existing assert_eq for lawHash) and add a symmetric assert_eq comparing metadata_json["schemaHash"] to generated_hash_literal(&generated, "WESLEY_SCHEMA_HASH").
🤖 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 `@CHANGELOG.md`:
- Around line 24-40: The changelog claims the CLI exposes the "wesley law"
command (e.g., "wesley law validate", and related emitted constants like
WESLEY_SCHEMA_HASH / WESLAW_HASH) but the current CLI preflight shows no "wesley
law" surface; update CHANGELOG.md to match shipped behavior by either removing
or qualifying those lines about "wesley law" and its emitted artifacts, or
explicitly note they are forthcoming (e.g., "planned / not yet shipped") until
the command and embedded constants are actually added; ensure references to
"wesley law validate", "WESLEY_SCHEMA_HASH", "WESLAW_HASH", and bundle/profile
emit behavior are consistent with current PRs before merging.
In `@crates/wesley-cli/src/main.rs`:
- Around line 749-755: The metadata builder currently always sets
EmitMetadata.schema_hash from compute_registry_hash(ir) which diverges from the
emit-with-law path that uses manifest.schema_hash; update the EmitMetadata
construction (where schema_hash is set) to use manifest.schema_hash when
manifest.is_some() (i.e., prefer manifest.map(|m| m.schema_hash.clone()) or
otherwise fallback to compute_registry_hash(ir)?), so the schema_hash in
EmitMetadata matches the manifest-derived schema hash used by emit-with-law;
locate EmitMetadata initialization in main.rs and adjust the schema_hash
assignment accordingly.
In `@docs/BEARING.md`:
- Around line 341-355: The PR description text is inconsistent with the updated
checklist in the BEARING.md diff; update the PR description to state "45 / 75"
slices closed (matching the updated status line that lists WLAW-001 through
WLAW-045) instead of the incorrect "35 / 75"; ensure any mention of the slice
total and the numeric progression referencing WLAW-021–045 reflects 45 / 75 so
the PR description and the docs/BEARING.md status line are consistent.
---
Nitpick comments:
In `@crates/wesley-cli/tests/cli.rs`:
- Around line 773-776: The test currently asserts metadata_json["lawHash"]
equals generated_hash_literal(&generated, "WESLAW_HASH") but does not assert the
schema hash; update the test to also assert metadata_json["schemaHash"] equals
generated_hash_literal(&generated, "WESLEY_SCHEMA_HASH") so the emitted schema
constant is validated. Locate the assertion using metadata_json and
generated_hash_literal in the test (around the existing assert_eq for lawHash)
and add a symmetric assert_eq comparing metadata_json["schemaHash"] to
generated_hash_literal(&generated, "WESLEY_SCHEMA_HASH").
In `@crates/wesley-emit-rust/src/lib.rs`:
- Around line 33-47: Add a crate-local unit test that calls
emit_rust_with_operations_and_hashes with a minimal WesleyIR and empty
operations plus sample schema_hash and law_hash strings, then assert the
returned String contains the emitted constants (look for the constant
identifiers or the exact hash values) and verify the output is valid Rust by
parsing it (e.g., with syn::parse_file or another Rust parser). Put the test in
the same crate (e.g., a #[cfg(test)] mod tests with #[test] fn
emit_hash_constants_present()), reference the public function
emit_rust_with_operations_and_hashes and the RustProvenanceConstants identifiers
to locate code, and fail the test if the constants are missing or the parse
fails.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54c6f088-2625-422f-b14a-744a26a69ce7
📒 Files selected for processing (18)
CHANGELOG.mdREADME.mdcrates/wesley-cli/src/main.rscrates/wesley-cli/tests/cli.rscrates/wesley-core/src/domain/law.rscrates/wesley-core/tests/law_ir.rscrates/wesley-emit-rust/src/lib.rsdocs/BEARING.mddocs/END_TO_END.mddocs/ENTRYPOINTS.mddocs/GUIDE.mddocs/design/0019-weslaw-semantic-law-ir/CANONICALIZATION_AND_DIAGNOSTICS.mddocs/design/0019-weslaw-semantic-law-ir/LAW_IR_V1.mddocs/design/0019-weslaw-semantic-law-ir/weslaw-semantic-law-ir.mdschemas/README.mdschemas/wesley-contract-bundle-manifest-v1.schema.jsontest/fixtures/weslaw/README.mdtest/weslaw-fixtures.bats
Owner
Author
|
@codex Self-review findings for this branch. Please confirm these before merge.
Summary: 4 findings total: P1=2, P2=1, P3=1. No whitespace or Markdown-style violations found by |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f2f021396
ℹ️ 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".
Owner
Author
Self-review resolution summaryResolved the strict self-review findings in commit
Validation run before and during push:
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/wesley-core/src/domain/law.rs (1)
1081-1083:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
validate_law_ir_v1_bindingsshould ignore draft entries.The function contract says active entries, but it validates/counts all entries. That can incorrectly fail bindings and inflate manifest
lawEntryCountwhen drafts are present.Suggested fix
- for (index, entry) in law_ir.entries.iter().enumerate() { + let active_entries = law_ir + .entries + .iter() + .enumerate() + .filter(|(_, entry)| entry.status == LawStatusV1::Active); + + let mut bound_entry_count = 0usize; + for (index, entry) in active_entries { let law_path = format!("$.laws[{index}]"); @@ context.bind_entry(entry, coordinate, &law_path)?; + bound_entry_count += 1; } @@ Ok(LawBindingReportV1 { schema_hash: active_schema_hash.to_string(), - bound_entry_count: law_ir.entries.len(), + bound_entry_count, }) }Also applies to: 1100-1103
🤖 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 `@crates/wesley-core/src/domain/law.rs` around lines 1081 - 1083, validate_law_ir_v1_bindings is iterating and counting all law_ir.entries but must only consider active (non-draft) entries; update the loops and any counts to filter entries by their status (e.g., skip entries where entry.status indicates Draft) before constructing law_path/subject_path and validating bindings so drafts are ignored when computing law entry counts and performing validations (apply same change to the other loop referenced around the later lines).
🤖 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 `@crates/wesley-cli/src/main.rs`:
- Around line 338-339: The code silently accepts unknown options.profile values
by defaulting to "release", allowing typos to bypass coverage gates; update the
handling around options.profile (where profile is set and passed to
law_coverage_report) to validate that the provided string is one of the allowed
values ("release" or "ci-release") and reject/exit with an error if it isn't;
apply the same validation pattern to the other similar profile-handling site
that passes a profile to law_coverage_report to ensure unknown profiles are
refused rather than treated as "release".
- Around line 1798-1806: The current logic counts raw occurrences of
old_schema_hash in source and does a global replace (using source.replace),
which is brittle; instead locate and replace only the schema.hash anchor line.
Change the code that uses source.matches(old_schema_hash).count() and
source.replace(old_schema_hash, new_schema_hash) to: search for a single line
that assigns schema.hash (e.g., pattern matching "schema.hash" followed by ":"
or "=" and the quoted hash), assert exactly one match, and replace only the hash
portion on that line with new_schema_hash (use the existing variables
old_schema_hash, new_schema_hash, source, and path to report errors if the
specific schema.hash line is missing or duplicated).
In `@docs/GUIDE.md`:
- Around line 48-54: The documented "wesley law" command entries (the lines
listing "Law lint", "Law", "Law Diff", "Law Explain", "Law Rebind", "Law
Capabilities", "Law Coverage" and the similar entries at 67-69) are inaccurate
because the native CLI does not expose a "wesley law" namespace; update
docs/GUIDE.md to either: (A) replace those command examples with the actual
native CLI command names and flags that are exposed by the codebase (so the
fast-path commands in GUIDE.md match the real CLI), or (B) remove the bogus
"wesley law ..." block and add a correct description pointing to the real CLI
entrypoints; ensure you update all occurrences noted (the block at the shown
diff and the entries at 67-69) and keep examples using the exact flag syntax the
native CLI exposes.
---
Outside diff comments:
In `@crates/wesley-core/src/domain/law.rs`:
- Around line 1081-1083: validate_law_ir_v1_bindings is iterating and counting
all law_ir.entries but must only consider active (non-draft) entries; update the
loops and any counts to filter entries by their status (e.g., skip entries where
entry.status indicates Draft) before constructing law_path/subject_path and
validating bindings so drafts are ignored when computing law entry counts and
performing validations (apply same change to the other loop referenced around
the later lines).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1e07b6aa-7b79-4e7e-bca0-ca1e407b7671
📒 Files selected for processing (24)
CHANGELOG.mdcrates/wesley-cli/src/main.rscrates/wesley-cli/tests/cli.rscrates/wesley-core/src/domain/law.rscrates/wesley-core/tests/law_ir.rscrates/wesley-emit-rust/src/lib.rsdocs/BEARING.mddocs/END_TO_END.mddocs/GUIDE.mddocs/design/0019-weslaw-semantic-law-ir/COORDINATES_AND_REGISTRIES.mddocs/design/0019-weslaw-semantic-law-ir/LAW_IR_V1.mddocs/design/0019-weslaw-semantic-law-ir/weslaw-semantic-law-ir.mdschemas/README.mdschemas/wesley-law-diff-v1.schema.jsontest/fixtures/weslaw/README.mdtest/fixtures/weslaw/accepted/channel-ttd-protocol-from-directive.weslaw.yamltest/fixtures/weslaw/accepted/rust-validator-payoff.weslaw.yamltest/fixtures/weslaw/diff/binding-broken.weslaw.yamltest/fixtures/weslaw/diff/ci-semantic-diff.jsontest/fixtures/weslaw/diff/ci-semantic-diff.mdtest/fixtures/weslaw/diff/holmes-blade-binding-broken.jsontest/fixtures/weslaw/diff/new.weslaw.yamltest/fixtures/weslaw/diff/old.weslaw.yamltest/weslaw-fixtures.bats
✅ Files skipped from review due to trivial changes (8)
- test/fixtures/weslaw/diff/ci-semantic-diff.md
- test/fixtures/weslaw/diff/binding-broken.weslaw.yaml
- docs/design/0019-weslaw-semantic-law-ir/LAW_IR_V1.md
- docs/design/0019-weslaw-semantic-law-ir/COORDINATES_AND_REGISTRIES.md
- docs/BEARING.md
- test/fixtures/weslaw/README.md
- docs/END_TO_END.md
- schemas/README.md
Owner
Author
|
Resolved review-thread follow-ups:
Validation after the fixes: |
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
weslaw/v1Law IRwesley law validate,lint,diff,explain,rebind,capabilities, andcoverageValidation
cargo test -p wesley-core --test law_ircargo test -p wesley-cli --test cliBATS_LIB_PATH=test/bats-plugins bats -t test/weslaw-fixtures.batspnpm run legacy-preflightpnpm run preflightgit diff --check