diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a235a6f..a050380 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -2,7 +2,7 @@ ## Project Overview -Geval is a **decision orchestration and reconciliation** tool for AI systems. It consumes **signals** (JSON) and **policy** (YAML), evaluates rules in priority order, and produces a deterministic decision: **PASS**, **REQUIRE_APPROVAL**, or **BLOCK**. It does not run evals, call APIs, or compute metrics—it only reconciles your rules against your signals. +Geval is a **decision orchestration and reconciliation** tool for AI systems. It consumes **signals** (JSON) and **policy** (YAML), evaluates **all** rules (unique priorities; **1** = highest), surfaces every match, applies the **best-priority** winner per policy, and merges policies/contracts with **`worst_case`** (BLOCK > REQUIRE_APPROVAL > PASS). It does not run evals, call APIs, or compute metrics—it only reconciles your rules against your signals. **Core Philosophy**: Geval has no “brain.” You provide signals and rules; Geval applies the rules and returns one outcome. Same inputs + same policy = same outcome. diff --git a/.github/workflows/geval.yml b/.github/workflows/geval.yml index 2330ee9..367ab6b 100644 --- a/.github/workflows/geval.yml +++ b/.github/workflows/geval.yml @@ -1,5 +1,4 @@ -# Geval Decision Check - run the Rust decision engine in CI -# Use this workflow to evaluate signals against policy on pull requests. +# Geval Decision Check - run `cargo test`, build release binary, evaluate example signals on PRs. name: Geval Decision Check @@ -24,6 +23,9 @@ jobs: echo "$HOME/.cargo/bin" >> $GITHUB_PATH shell: bash + - name: Test Geval + run: cargo test --manifest-path geval/Cargo.toml + - name: Build Geval run: cargo build --release --manifest-path geval/Cargo.toml diff --git a/README.md b/README.md index 83da34b..26013b5 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ You can add labels like `component` or `system` if you need them. [Full example ### Step 2: Your contract and policies -A **contract** is a YAML file that lists one or more **policy** files and a **combination rule** (how to merge their outcomes). Each **policy** file contains ordered rules: **When** [condition on signals], **then** [pass / block / require_approval]. +A **contract** is a YAML file that lists one or more **policy** files and a **combination rule** (how to merge their outcomes). Each **policy** file contains rules with **unique** priorities: **When** [condition on signals], **then** [pass / block / require_approval]. **Prefer a form instead of writing YAML by hand?** Use **[config.geval.io](https://config.geval.io)** to generate Geval-compatible `contract.yaml` and policy files (download or copy), then validate with `geval validate-contract` and run `geval check` as below. @@ -113,7 +113,7 @@ Example contract — save as `contract.yaml`: ```yaml name: my-gate version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: policy.yaml ``` @@ -143,7 +143,7 @@ policy: action: pass ``` -**Combine rules:** `all_pass` = PASS only if every policy passes; `any_block_blocks` = any policy BLOCK → overall BLOCK. **Operators:** `>`, `<`, `>=`, `<=`, `==`, `presence`. **Actions:** `pass`, `block`, `require_approval`. +**Combine (`worst_case`):** any **BLOCK** wins; else any **require_approval**; else **pass**. **Rule priorities** must be **unique** per policy; **1** = highest; Geval records every match and the **best** priority wins. **Operators:** `>`, `<`, `>=`, `<=`, `==`, `presence`. **Actions:** `pass`, `block`, `require_approval`. [Full example →](geval/examples/contract.yaml) and [policy →](geval/examples/policy.yaml) diff --git a/geval/README.md b/geval/README.md index c38e577..286705a 100644 --- a/geval/README.md +++ b/geval/README.md @@ -16,7 +16,7 @@ See **[Installation](docs/installation.md)** for download links, build-from-sour ## Principles - **Local, deterministic** – single binary, no external services -- **Rule-based** – priority-ordered rules; first match wins; no scoring or ML +- **Rule-based** – unique priorities per policy (**1** = highest); all matches shown; best priority wins; no scoring or ML - **Auditable** – policy and signal hashes (SHA256), immutable decision artifacts ## Quick start diff --git a/geval/docs/architecture.md b/geval/docs/architecture.md index ae07575..aa6ad79 100644 --- a/geval/docs/architecture.md +++ b/geval/docs/architecture.md @@ -36,7 +36,7 @@ Geval is the **thin layer** that sits between **evidence** (signals) and **polic ```mermaid flowchart LR - subgraph legacyBefore [Before Geval] + subgraph beforeGeval [Before Geval] direction TB mixed[Non-uniform signals — numbers flags presence labels components AB KPIs] discuss[Slack and meetings — people interpret and debate] @@ -244,16 +244,16 @@ flowchart TB | **Contract** | YAML file: `name`, `version`, `combine` (rule), and list of policy paths. The unit of evaluation. | | **Policy** | YAML file: optional `name`/`version`, `environment`, and ordered `rules`. Each rule has `when` (conditions) and `then` (action: pass / block / require_approval). | | **Signals** | JSON: optional `name`/`version`, and array of signal objects (metric, value, component, etc.). Facts fed into the engine. | -| **Combination rule** | How to merge outcomes from multiple policies: `all_pass` or `any_block_blocks`. | +| **Combination rule** | How to merge outcomes from multiple policies/contracts: **`worst_case`** — BLOCK > REQUIRE_APPROVAL > PASS. | ## Data flow 1. **Load contract** – Parse contract YAML; resolve policy paths relative to the contract file; load each policy. 2. **Load signals** – Parse signals JSON; build an in-memory signal graph (metric → value lookup). -3. **Evaluate each policy** – For each policy, evaluate rules in priority order; first matching rule gives that policy’s outcome (PASS / REQUIRE_APPROVAL / BLOCK). -4. **Combine (policies)** – Apply the contract’s combination rule to the list of policy outcomes → one combined decision **per contract**. -5. **Combine (contracts)** – If multiple contract files are passed (`geval check -c a.yaml -c b.yaml`), apply **`--combine-contracts`** to each contract’s combined outcome → one **overall** PR-level decision (same rule vocabulary: `all_pass`, `any_block_blocks`). -6. **Artifact** – Write `.geval/decisions/.json` (v3) with `bundle_hash`, each contract block, `contracts_combine_rule`, and overall outcome + hashes. +3. **Evaluate each policy** – For each policy, **every** rule is checked against the signal graph. **All** matches are recorded; the **winning** rule is the one with the **best** priority (**1** = highest; larger numbers are lower). That rule’s action is the policy outcome (PASS / REQUIRE_APPROVAL / BLOCK). **Priorities must be unique** within a policy (validated at load). +4. **Combine (policies)** – Apply **`worst_case`** merging to the list of policy outcomes → one combined decision **per contract**. +5. **Combine (contracts)** – If multiple contract files are passed (`geval check -c a.yaml -c b.yaml`), apply **`--combine-contracts`** (same **`worst_case`** semantics) to each contract’s combined outcome → one **overall** PR-level decision. +6. **Artifact** – Write `.geval/decisions/.json` (v4) with `bundle_hash`, each contract block, `contracts_combine_rule`, per-policy `matching_rules`, and overall outcome + hashes. ## Module layout @@ -261,7 +261,7 @@ flowchart TB geval/src/ contract/ # Contract = multiple policies + combine rule model.rs # ContractDef, PolicyRef - combine.rs # CombineRule (all_pass, any_block_blocks), apply_combine_rule + combine.rs # CombineRule (worst_case), apply_combine_rule loader.rs # load_contract, load_contract_and_policies, parse_contract_str runner.rs # run_contract, load_run_contracts → ContractResult / MultiContractRun policy/ # Single policy model and parser @@ -272,7 +272,7 @@ geval/src/ signal_graph/ # Build lookup from signals for rule matching signals/ # Load signals JSON (name, version, signals array) hashing/ # SHA256 for contract, policy, signals, contract bundle (audit) - artifact/ # write_multi_contract_artifact (v3: multi-contract + overall) + artifact/ # write_multi_contract_artifact (v4: multi-contract + overall) explanation/ # explain_contract_result, explain_multi_contract_result, explain_decision approval/ # Approval/rejection artifact (versioned) cli/ # Commands: check, init, demo, explain, validate-contract, approve, reject diff --git a/geval/docs/auditing.md b/geval/docs/auditing.md index 285e3c7..b997879 100644 --- a/geval/docs/auditing.md +++ b/geval/docs/auditing.md @@ -4,7 +4,7 @@ Geval is designed so **nothing is unversioned**: every decision and every action - **Why was this deployed?** – Decision report and matched rule (and optional approval reason). - **Who approved it?** – `geval approve` writes an artifact with `approved_by`, `reason`, and artifact `version`. -- **What policy (contract) was used?** – Artifact stores each contract’s identity, `contract_hash`, per-policy hashes, and (v3) `bundle_hash` for the ordered set of contracts. +- **What policy (contract) was used?** – Artifact stores each contract’s identity, `contract_hash`, per-policy hashes, and `bundle_hash` for the ordered set of contracts. - **What signals were used?** – Artifact stores `signals_name`, `signals_version`, and `signals_hash` (SHA256). - **Which Geval binary?** – Artifact stores `geval_version`. @@ -17,22 +17,22 @@ Geval is designed so **nothing is unversioned**: every decision and every action Each `geval check` run writes: - **Path:** `.geval/decisions/.json` -- **Contents (artifact_version 3, multi-contract):** - - `artifact_version` – schema version (`"3"`) +- **Contents (artifact_version 4, multi-contract):** + - `artifact_version` – schema version (`"4"`) - `geval_version` – binary version that produced the decision - `bundle_hash` – SHA256 over the ordered list of `(contract_path, contract_hash)` (audit the exact contract set) - - `contracts_combine_rule` – how each contract’s **combined** outcome was merged (e.g. `all_pass`, `any_block_blocks`) + - `contracts_combine_rule` – how each contract’s **combined** outcome was merged (for example `worst_case`) - `contracts` – array of blocks, each with: - `contract_path`, `contract_name`, `contract_version`, `contract_hash` - `combine_rule` (policies within that contract) - - `policy_results` – `{ policy_path, policy_name?, policy_version?, policy_hash, outcome, matched_rule? }[]` + - `policy_results` – `{ policy_path, policy_name?, policy_version?, policy_hash, outcome, matched_rule?, matching_rules? }[]` (`matching_rules` lists every rule whose `when` matched, in priority order; `matched_rule` is the winner) - `combined_decision`, `combined_matched_rule`, `combined_reason` (outcome for that contract) - `overall_combined_decision`, `overall_matched_rule`, `overall_reason` – PR-level outcome after `contracts_combine_rule` - `signals_name`, `signals_version`, `signals_hash` – signals identity and content hash - `timestamp` – ISO8601 - `approval` – optional; set when an approval is recorded for this decision -Older tooling may still reference **artifact_version 2** (single flat contract); Geval now writes v3 only. +Older tooling may still reference **artifact_version 2** or **3**; current Geval writes **v4** (adds `matching_rules` per policy). ### Approval artifact diff --git a/geval/docs/customer-demo-feature.md b/geval/docs/customer-demo-feature.md index 5c3f869..9fe2ca5 100644 --- a/geval/docs/customer-demo-feature.md +++ b/geval/docs/customer-demo-feature.md @@ -32,7 +32,7 @@ These are **inputs** (your pipeline or eval harness writes one `signals.json` pe ## 3. Policies — meaningful split (and what to say) -Use **separate policy files** so **ownership** is clear (security vs product vs business). The **contract** lists them and sets **`combine: all_pass`** so: *every policy must pass; any BLOCK wins; REQUIRE_APPROVAL without BLOCK means “needs a human”.* +Use **separate policy files** so **ownership** is clear (security vs product vs business). The **contract** lists them and sets **`combine: worst_case`** so outcomes merge by severity: *any **BLOCK** wins; else any **REQUIRE_APPROVAL**; else **PASS**.* | Policy file | Owner (story) | Why separate | |-------------|---------------|--------------| @@ -47,7 +47,7 @@ Use **separate policy files** so **ownership** is clear (security vs product vs ## 4. Rules customers actually write (examples + why) -Rules are **ordered**; **first match wins**. Priorities below are **intentional** (stop fast on catastrophes, then quality, then business). +Rules use **unique** priorities (**`1`** = highest precedence). Geval shows **every** rule whose condition matched; the **winning** rule is the one with the **best** priority. The table below orders rules by priority on purpose (catastrophes first, then quality, then business). ### 4.1 `policies/safety.yaml` @@ -78,7 +78,7 @@ Rules are **ordered**; **first match wins**. Priorities below are **intentional* ```yaml name: support-copilot-release-gate version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: policies/safety.yaml - path: policies/product_quality.yaml @@ -86,7 +86,7 @@ policies: ``` **Customer line:** -*`all_pass`* means: every policy must end in PASS for an overall PASS; any policy BLOCK → overall BLOCK; if no BLOCK but something needs approval → overall REQUIRE_APPROVAL. +*`worst_case`* means: merge policy outcomes by severity — any **BLOCK** → overall **BLOCK**; else any **REQUIRE_APPROVAL** → overall **REQUIRE_APPROVAL**; else **PASS**. --- @@ -94,8 +94,8 @@ policies: 1. **CI (or a human) runs** your evals and **writes `signals.json`** with the metrics above (and optional `name` / `version` on the file for audit). 2. **Geval loads** the contract → loads the three policies → builds a small **lookup** from signals. -3. **Per policy**, rules run in **priority order**; the **first** rule whose `when` matches decides that policy’s outcome. -4. **Per policy outcomes** are merged with the contract’s **`combine`** rule → **one** outcome for the run. +3. **Per policy**, **every** rule is checked. **All** matches are listed; the rule with the **best** priority (**`1`** highest) **wins** and sets that policy’s outcome. +4. **Per policy outcomes** are merged with **`worst_case`** (same severity order: BLOCK > REQUIRE_APPROVAL > PASS) → **one** outcome for the contract run. 5. **Geval exits** with 0 / 1 / 2 and can write a **decision artifact** (who/what/when + hashes). **Customer line:** diff --git a/geval/docs/extending.md b/geval/docs/extending.md index 4d318f7..f824dd3 100644 --- a/geval/docs/extending.md +++ b/geval/docs/extending.md @@ -45,15 +45,11 @@ Run: `cargo test --manifest-path geval/Cargo.toml`. - Bump version in `geval/Cargo.toml` and, if needed, `DECISION_ARTIFACT_VERSION` or `APPROVAL_ARTIFACT_VERSION`. - Note breaking changes (e.g. CLI now requires `--contract` instead of `--policy`) in release notes. -## Adding a new combination rule - -1. **contract/combine.rs** - - Add a variant to `CombineRule` with `#[serde(rename = "snake_case")]` (or explicit rename). - - Implement `Default` if it should be the default when omitted in YAML. - - In `apply_combine_rule`, add a `match` branch that implements the new semantics. - - Implement `Display` and `FromStr` for CLI/artifact string. -2. **Tests** – Add tests in `contract/combine::tests` for the new rule (e.g. N outcomes → expected combined outcome). -3. **Docs** – Update [signals-and-rules.md](signals-and-rules.md) or [versioning.md](versioning.md) to describe the new rule. +## Combination rules + +Today there is **one** merge semantics: **`worst_case`** (BLOCK > REQUIRE_APPROVAL > PASS), implemented in `contract/combine.rs`. + +To add a **different** combination mode in the future: add a `CombineRule` variant, implement it in `apply_combine_rule`, extend `Display` / `FromStr`, add tests, and bump `DECISION_ARTIFACT_VERSION` if artifact strings change. ## Adding a new policy or contract field diff --git a/geval/docs/github-actions.md b/geval/docs/github-actions.md index 939d7df..a5c5553 100644 --- a/geval/docs/github-actions.md +++ b/geval/docs/github-actions.md @@ -40,7 +40,7 @@ jobs: --env prod ``` -Repeat `--contract` for each gate YAML attached to the PR. Optional: `--combine-contracts all_pass` (default) or `any_block_blocks`. +Repeat `--contract` for each gate YAML attached to the PR. Optional: `--combine-contracts worst_case` (default). ## Option B: Download released binary diff --git a/geval/docs/signals-and-rules.md b/geval/docs/signals-and-rules.md index cebc137..5185f3e 100644 --- a/geval/docs/signals-and-rules.md +++ b/geval/docs/signals-and-rules.md @@ -32,6 +32,10 @@ So: - **Signals with scores** use the usual comparison operators. Example: “If `accuracy` < 0.9 → block.” - You can combine both in one policy: some rules key off presence, others off numeric thresholds. +### Priorities within a policy + +Each rule has a numeric **`priority`**. **Lower numbers are higher precedence: `1` is the highest.** Every priority must be **unique** within a policy (Geval rejects duplicate values when loading YAML). Geval evaluates **every** rule, records **all** that match, and the **winning** rule is the match with the **best** (numerically smallest) priority; that rule’s `then` action is the policy outcome. + ## Example: mixed signals **signals.json:** diff --git a/geval/docs/versioning.md b/geval/docs/versioning.md index 06557cd..cb346e5 100644 --- a/geval/docs/versioning.md +++ b/geval/docs/versioning.md @@ -14,7 +14,7 @@ Example: ```yaml name: release-gate version: "2.1.0" -combine: all_pass +combine: worst_case policies: - path: policies/security.yaml - path: policies/quality.yaml diff --git a/geval/examples/README.md b/geval/examples/README.md index e436069..92813c2 100644 --- a/geval/examples/README.md +++ b/geval/examples/README.md @@ -37,8 +37,7 @@ With the example data, the policy matches `business_block`: `engagement_drop` 0. - **name**, **version** – Identify the contract for audit; bump version when you change policies or combine rule. - **combine** – How to merge outcomes from multiple policies: - - **all_pass** – PASS only if every policy passes; any BLOCK → BLOCK; any REQUIRE_APPROVAL (no BLOCK) → REQUIRE_APPROVAL. - - **any_block_blocks** – Any policy BLOCK → overall BLOCK; else any REQUIRE_APPROVAL → REQUIRE_APPROVAL; else PASS. + - **worst_case** – Any BLOCK wins; else any REQUIRE_APPROVAL; else PASS. - **policies** – List of policy file paths (relative to the contract file): e.g. `policy.yaml` or `policies/security.yaml`. ## Policy format @@ -46,9 +45,9 @@ With the example data, the policy matches `business_block`: `engagement_drop` 0. Each policy file has optional **name** and **version**, and **policy** with: - **environment** – optional. -- **rules** – priority, name, when (metric, component, operator, threshold), then (action, reason). +- **rules** – unique **priority** (**1** = highest), name, when (metric, component, operator, threshold), then (action, reason). -First matching rule wins; no match → PASS. +Every rule is evaluated; all matches are recorded; the **best** (lowest) priority wins; no match → PASS. ## Signals format diff --git a/geval/examples/contract-b.yaml b/geval/examples/contract-b.yaml index 7f2210c..c1a92f3 100644 --- a/geval/examples/contract-b.yaml +++ b/geval/examples/contract-b.yaml @@ -1,6 +1,6 @@ # Second example contract: same policy file, distinct contract identity (multi-contract PR demo). name: demo-secondary version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: policy.yaml diff --git a/geval/examples/contract.yaml b/geval/examples/contract.yaml index 1cb47b0..691b2fe 100644 --- a/geval/examples/contract.yaml +++ b/geval/examples/contract.yaml @@ -3,6 +3,6 @@ name: demo version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: policy.yaml diff --git a/geval/src/artifact/writer.rs b/geval/src/artifact/writer.rs index 6329072..e5c192b 100644 --- a/geval/src/artifact/writer.rs +++ b/geval/src/artifact/writer.rs @@ -1,7 +1,7 @@ //! Write decision artifacts to .geval/decisions/.json //! -//! Multi-contract: artifact v3 records each contract (path, hashes, per-policy results) plus -//! `contracts_combine_rule`, `bundle_hash`, and overall PR-level decision. +//! Multi-contract: artifact v4 adds `matching_rules` per policy; `contracts_combine_rule` records +//! the merge mode (typically `worst_case`). use crate::contract::MultiContractRun; use crate::evaluator::DecisionOutcome; @@ -13,7 +13,7 @@ use std::path::Path; use crate::hashing::hash_contract_bundle; /// Schema version of the decision artifact format. Bump when the artifact shape changes. -pub const DECISION_ARTIFACT_VERSION: &str = "3"; +pub const DECISION_ARTIFACT_VERSION: &str = "4"; /// Per-policy result as stored in the artifact. #[derive(Debug, Serialize)] @@ -27,6 +27,8 @@ pub struct PolicyResultRecord { pub outcome: String, #[serde(skip_serializing_if = "Option::is_none")] pub matched_rule: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub matching_rules: Vec, } /// One contract’s slice of the artifact (mirrors former v2 single-contract payload, nested). @@ -45,7 +47,7 @@ pub struct ContractDecisionBlock { pub combined_reason: Option, } -/// Multi-contract decision artifact (v3). +/// Multi-contract decision artifact (v4). #[derive(Debug, Serialize)] pub struct DecisionArtifactV3 { pub artifact_version: String, @@ -89,6 +91,7 @@ fn policy_records_for_contract( policy_hash: hash.clone(), outcome: outcome_str(r.outcome).to_string(), matched_rule: r.matched_rule.clone(), + matching_rules: r.matching_rules.clone(), }) .collect() } @@ -200,7 +203,7 @@ mod tests { format!( r#"name: {} version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: {} "#, @@ -211,7 +214,7 @@ policies: } #[test] - fn write_multi_contract_artifact_is_valid_v3_json() { + fn write_multi_contract_artifact_is_valid_v4_json() { let dir = tempfile::tempdir().unwrap(); minimal_pass_contract(dir.path(), "a", "ca.yaml", "pa.yaml"); minimal_pass_contract(dir.path(), "b", "cb.yaml", "pb.yaml"); @@ -220,7 +223,7 @@ policies: let signals = SignalSet::new(vec![sig("x", 1.0)]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&[c1, c2], &graph, CombineRule::AllPass).unwrap(); + let run = load_run_contracts(&[c1, c2], &graph, CombineRule::WorstCase).unwrap(); let out_dir = tempfile::tempdir().unwrap(); let path = write_multi_contract_artifact( @@ -235,8 +238,8 @@ policies: let json = std::fs::read_to_string(&path).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); - assert_eq!(v["artifact_version"], "3"); - assert_eq!(v["contracts_combine_rule"], "all_pass"); + assert_eq!(v["artifact_version"], "4"); + assert_eq!(v["contracts_combine_rule"], "worst_case"); assert_eq!(v["overall_combined_decision"], "PASS"); assert!(v["bundle_hash"].as_str().unwrap().len() == 64); assert_eq!(v["contracts"].as_array().unwrap().len(), 2); @@ -268,7 +271,7 @@ policies: &c, r#"name: solo version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: policy.yaml "#, @@ -277,7 +280,7 @@ policies: let signals = SignalSet::new(vec![sig("x", 99.0)]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&[c], &graph, CombineRule::AllPass).unwrap(); + let run = load_run_contracts(&[c], &graph, CombineRule::WorstCase).unwrap(); assert_eq!(run.entries.len(), 1); let out_dir = tempfile::tempdir().unwrap(); @@ -289,6 +292,10 @@ policies: let pr = contracts[0]["policy_results"].as_array().unwrap(); assert_eq!(pr.len(), 1); assert_eq!(pr[0]["outcome"], "BLOCK"); + assert_eq!( + pr[0]["matching_rules"].as_array().unwrap()[0].as_str().unwrap(), + "block_x" + ); assert_eq!(v["overall_combined_decision"], "BLOCK"); } @@ -299,7 +306,7 @@ policies: let c = dir.path().join("c.yaml"); let signals = SignalSet::new(vec![sig("x", 1.0)]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&[c], &graph, CombineRule::AllPass).unwrap(); + let run = load_run_contracts(&[c], &graph, CombineRule::WorstCase).unwrap(); let out_dir = tempfile::tempdir().unwrap(); let approval = ApprovalPayload { approved_by: "alice".to_string(), diff --git a/geval/src/cli/commands.rs b/geval/src/cli/commands.rs index 4b10398..831881b 100644 --- a/geval/src/cli/commands.rs +++ b/geval/src/cli/commands.rs @@ -118,10 +118,10 @@ pub struct CheckOpts { /// Contract YAML file(s); repeat for multiple contracts on one PR. #[arg(long, short = 'c', action = clap::ArgAction::Append, required = true)] pub contract: Vec, - /// How to merge each contract’s combined outcome (default: all must pass). + /// How to merge each contract’s combined outcome (default: worst_case — BLOCK > REQUIRE_APPROVAL > PASS). #[arg( long = "combine-contracts", - default_value = "all_pass", + default_value = "worst_case", value_parser = parse_combine_rule )] pub combine_contracts: CombineRule, @@ -159,7 +159,7 @@ pub struct ExplainOpts { pub contract: Vec, #[arg( long = "combine-contracts", - default_value = "all_pass", + default_value = "worst_case", value_parser = parse_combine_rule )] pub combine_contracts: CombineRule, @@ -230,7 +230,7 @@ fn run_demo(opts: &DemoOpts) -> Result<()> { let contract = ContractDef { name: "demo".to_string(), version: "1.0.0".to_string(), - combine: CombineRule::AllPass, + combine: CombineRule::WorstCase, policies: vec![PolicyRef { path: "demo.yaml".to_string(), }], @@ -246,6 +246,7 @@ fn run_demo(opts: &DemoOpts) -> Result<()> { "policy_path": r.policy_path, "outcome": outcome_str(r.outcome), "matched_rule": r.matched_rule, + "matching_rules": r.matching_rules, })).collect::>(), }); println!("{}", serde_json::to_string_pretty(&out)?); @@ -301,6 +302,7 @@ fn run_check(opts: &CheckOpts) -> Result<()> { "policy_path": r.policy_path, "outcome": outcome_str(r.outcome), "matched_rule": r.matched_rule, + "matching_rules": r.matching_rules, })).collect::>(), "combined_decision": outcome_str(e.result.combined_decision.outcome), })).collect::>(), diff --git a/geval/src/cli/demo_ui.rs b/geval/src/cli/demo_ui.rs index 45e7858..83b4ec8 100644 --- a/geval/src/cli/demo_ui.rs +++ b/geval/src/cli/demo_ui.rs @@ -237,7 +237,7 @@ pub fn print_demo_report( loading_then(&mut out, &loading1, &done1, DELAY_LOAD); line(&mut out, DELAY_LINE, &format!(" {} {}", d(g.box_v), d("Environment:"))); line(&mut out, DELAY_LINE, &format!(" {} {} {}", d(g.box_v), d(" "), environment.unwrap_or("(not set)"))); - line(&mut out, DELAY_LINE, &format!(" {} {}", d(g.box_v), d("Rules (evaluated in priority order):"))); + line(&mut out, DELAY_LINE, &format!(" {} {}", d(g.box_v), d("Rules (priority 1 = highest; each priority unique):"))); for (i, rule) in policy.sorted_rules().iter().enumerate() { line(&mut out, DELAY_LINE, &format!(" {} {} {}. {} {} {}", d(g.box_v), d(" "), i + 1, magenta_s(&rule.name), d(g.arrow_act), d(action_str(rule.then.action)))); } @@ -256,7 +256,7 @@ pub fn print_demo_report( // Step 3: Rules let loading3 = format!(" {} {}", green_s(g.step), d("Evaluating rules...")); - let done3 = format!(" {} {}", green_s(g.step), b("Step 3: Evaluating rules (first match wins)")); + let done3 = format!(" {} {}", green_s(g.step), b("Step 3: All rules checked; best priority wins")); loading_then(&mut out, &loading3, &done3, DELAY_LOAD); let traced_names: std::collections::HashSet<_> = trace.iter().map(|t| t.rule_name.as_str()).collect(); let sorted = policy.sorted_rules(); @@ -307,8 +307,24 @@ pub fn print_demo_report( line(&mut out, DELAY_LINE, &format!(" {} {}", d(g.box_v), d(""))); line(&mut out, DELAY_LINE, &format!(" {} {} {}", d(g.box_v), d("Reason:"), reason)); } + if !decision.matching_rules.is_empty() { + line( + &mut out, + DELAY_LINE, + &format!( + " {} {} {}", + d(g.box_v), + d("Rules that matched:"), + decision.matching_rules.join(", ") + ), + ); + } if let Some(ref name) = decision.matched_rule { - line(&mut out, DELAY_LINE, &format!(" {} {} {}", d(g.box_v), d("Matched rule:"), name)); + line( + &mut out, + DELAY_LINE, + &format!(" {} {} {}", d(g.box_v), d("Winning rule (best priority):"), name), + ); } line(&mut out, 0, ""); let _ = out.flush(); diff --git a/geval/src/cli/init.rs b/geval/src/cli/init.rs index 87a4c37..67a0694 100644 --- a/geval/src/cli/init.rs +++ b/geval/src/cli/init.rs @@ -40,11 +40,11 @@ const SIGNALS_TEMPLATE: &str = r#"{ const CONTRACT_TEMPLATE: &str = r#"# Geval contract: multiple policies evaluated together. # name + version identify this contract; bump version when you add/remove policies or change combine. -# combine: all_pass = PASS only if every policy passes; any_block_blocks = any BLOCK → overall BLOCK. +# combine: worst_case merges outcomes by severity — BLOCK > REQUIRE_APPROVAL > PASS. name: release-gate version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: policies/security.yaml - path: policies/quality.yaml @@ -120,10 +120,9 @@ Created by `geval init`. Edit the files in this folder and run Geval from your p - **policies/** — Policy files (e.g. security.yaml, quality.yaml). Each has name, version, and rules. Paths in contract are relative to the contract file. - **signals.json** — Your data (metrics, scores). Set name and version; bump version when the pipeline or schema changes. -## Combine rules +## Combine rule (`combine`) -- **all_pass** — Overall PASS only if every policy returns PASS; any BLOCK → BLOCK; any REQUIRE_APPROVAL (and no BLOCK) → REQUIRE_APPROVAL. -- **any_block_blocks** — Any policy BLOCK → overall BLOCK; else any REQUIRE_APPROVAL → REQUIRE_APPROVAL; else PASS. +- **worst_case** — Merge by severity: any **BLOCK** wins; else any **REQUIRE_APPROVAL**; else **PASS**. ## Run diff --git a/geval/src/contract/combine.rs b/geval/src/contract/combine.rs index c272284..10ab765 100644 --- a/geval/src/contract/combine.rs +++ b/geval/src/contract/combine.rs @@ -1,27 +1,28 @@ //! Combination rules: how multiple policy outcomes are merged into one contract decision. //! -//! Extensible: add a new variant to `CombineRule` and implement the logic in `apply`. +//! Geval uses a **single** merge semantics everywhere: **worst outcome wins** — +//! `BLOCK` beats `REQUIRE_APPROVAL` beats `PASS`. Contract YAML uses `combine: worst_case`. use crate::evaluator::DecisionOutcome; use serde::{Deserialize, Serialize}; /// How to combine outcomes from multiple policies into a single contract decision. +/// +/// Only one rule exists today: merge by severity (worst wins). Additional accepted spellings +/// are handled via `#[serde(alias = ...)]` for compatibility with existing files. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum CombineRule { + /// Any `BLOCK` wins; else any `REQUIRE_APPROVAL`; else `PASS`. #[default] - /// PASS only if every policy returns PASS. Otherwise: any BLOCK → BLOCK; else REQUIRE_APPROVAL. - AllPass, - - /// If any policy returns BLOCK → BLOCK; else if any REQUIRE_APPROVAL → REQUIRE_APPROVAL; else PASS. - AnyBlockBlocks, + #[serde(alias = "all_pass", alias = "any_block_blocks")] + WorstCase, } impl std::fmt::Display for CombineRule { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - CombineRule::AllPass => write!(f, "all_pass"), - CombineRule::AnyBlockBlocks => write!(f, "any_block_blocks"), + CombineRule::WorstCase => write!(f, "worst_case"), } } } @@ -31,43 +32,27 @@ impl std::str::FromStr for CombineRule { fn from_str(s: &str) -> Result { match s.trim().to_lowercase().as_str() { - "all_pass" => Ok(CombineRule::AllPass), - "any_block_blocks" => Ok(CombineRule::AnyBlockBlocks), - _ => Err(format!("unknown combine rule: {}", s)), + "worst_case" | "all_pass" | "any_block_blocks" => Ok(CombineRule::WorstCase), + _ => Err(format!("unknown combine rule: {} (expected worst_case)", s)), } } } -/// Apply the combination rule to a slice of policy outcomes (order preserved). -/// Returns the single contract-level outcome. -pub fn apply_combine_rule(rule: CombineRule, outcomes: &[DecisionOutcome]) -> DecisionOutcome { +/// Merge policy or contract outcomes: **BLOCK** > **REQUIRE_APPROVAL** > **PASS**. +/// The `rule` argument is kept for API stability; behavior does not depend on it. +pub fn apply_combine_rule(_rule: CombineRule, outcomes: &[DecisionOutcome]) -> DecisionOutcome { if outcomes.is_empty() { return DecisionOutcome::Pass; } - match rule { - CombineRule::AllPass => { - let any_block = outcomes.iter().any(|o| *o == DecisionOutcome::Block); - let any_approval = outcomes.iter().any(|o| *o == DecisionOutcome::RequireApproval); - if any_block { - DecisionOutcome::Block - } else if any_approval { - DecisionOutcome::RequireApproval - } else { - DecisionOutcome::Pass - } - } - CombineRule::AnyBlockBlocks => { - if outcomes.iter().any(|o| *o == DecisionOutcome::Block) { - DecisionOutcome::Block - } else if outcomes - .iter() - .any(|o| *o == DecisionOutcome::RequireApproval) - { - DecisionOutcome::RequireApproval - } else { - DecisionOutcome::Pass - } - } + if outcomes.iter().any(|o| *o == DecisionOutcome::Block) { + DecisionOutcome::Block + } else if outcomes + .iter() + .any(|o| *o == DecisionOutcome::RequireApproval) + { + DecisionOutcome::RequireApproval + } else { + DecisionOutcome::Pass } } @@ -76,69 +61,75 @@ mod tests { use super::*; #[test] - fn all_pass_all_pass() { + fn worst_case_all_pass_outcomes() { let outcomes = [ DecisionOutcome::Pass, DecisionOutcome::Pass, DecisionOutcome::Pass, ]; - assert_eq!(apply_combine_rule(CombineRule::AllPass, &outcomes), DecisionOutcome::Pass); + assert_eq!( + apply_combine_rule(CombineRule::WorstCase, &outcomes), + DecisionOutcome::Pass + ); } #[test] - fn all_pass_any_block() { + fn worst_case_block_beats_pass() { let outcomes = [ DecisionOutcome::Pass, DecisionOutcome::Block, DecisionOutcome::Pass, ]; - assert_eq!(apply_combine_rule(CombineRule::AllPass, &outcomes), DecisionOutcome::Block); + assert_eq!( + apply_combine_rule(CombineRule::WorstCase, &outcomes), + DecisionOutcome::Block + ); } #[test] - fn all_pass_any_approval_no_block() { + fn worst_case_require_approval_when_no_block() { let outcomes = [ DecisionOutcome::Pass, DecisionOutcome::RequireApproval, DecisionOutcome::Pass, ]; assert_eq!( - apply_combine_rule(CombineRule::AllPass, &outcomes), + apply_combine_rule(CombineRule::WorstCase, &outcomes), DecisionOutcome::RequireApproval ); } #[test] - fn any_block_blocks_none() { + fn worst_case_two_passes() { let outcomes = [DecisionOutcome::Pass, DecisionOutcome::Pass]; assert_eq!( - apply_combine_rule(CombineRule::AnyBlockBlocks, &outcomes), + apply_combine_rule(CombineRule::WorstCase, &outcomes), DecisionOutcome::Pass ); } #[test] - fn any_block_blocks_one_block() { + fn worst_case_block_beats_require_approval() { let outcomes = [ DecisionOutcome::Pass, DecisionOutcome::Block, DecisionOutcome::RequireApproval, ]; assert_eq!( - apply_combine_rule(CombineRule::AnyBlockBlocks, &outcomes), + apply_combine_rule(CombineRule::WorstCase, &outcomes), DecisionOutcome::Block ); } #[test] - fn any_block_blocks_approval_only() { + fn worst_case_require_approval_beats_pass() { let outcomes = [ DecisionOutcome::Pass, DecisionOutcome::RequireApproval, DecisionOutcome::Pass, ]; assert_eq!( - apply_combine_rule(CombineRule::AnyBlockBlocks, &outcomes), + apply_combine_rule(CombineRule::WorstCase, &outcomes), DecisionOutcome::RequireApproval ); } @@ -146,12 +137,30 @@ mod tests { #[test] fn empty_outcomes_pass() { assert_eq!( - apply_combine_rule(CombineRule::AllPass, &[]), + apply_combine_rule(CombineRule::WorstCase, &[]), DecisionOutcome::Pass ); + } + + #[test] + fn from_str_accepts_worst_case_and_equivalent_spellings() { + assert_eq!( + "worst_case".parse::().unwrap(), + CombineRule::WorstCase + ); assert_eq!( - apply_combine_rule(CombineRule::AnyBlockBlocks, &[]), - DecisionOutcome::Pass + "all_pass".parse::().unwrap(), + CombineRule::WorstCase ); + assert_eq!( + "any_block_blocks".parse::().unwrap(), + CombineRule::WorstCase + ); + assert!("nope".parse::().is_err()); + } + + #[test] + fn display_is_worst_case() { + assert_eq!(CombineRule::WorstCase.to_string(), "worst_case"); } } diff --git a/geval/src/contract/loader.rs b/geval/src/contract/loader.rs index cf1ca0f..34b06c4 100644 --- a/geval/src/contract/loader.rs +++ b/geval/src/contract/loader.rs @@ -82,7 +82,7 @@ mod tests { let yaml = r#" name: release-gate version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: security.yaml - path: quality.yaml @@ -90,11 +90,25 @@ policies: let c = parse_contract_str(yaml).unwrap(); assert_eq!(c.name, "release-gate"); assert_eq!(c.version, "1.0.0"); + assert_eq!(c.combine, crate::contract::CombineRule::WorstCase); assert_eq!(c.policies.len(), 2); assert_eq!(c.policies[0].path, "security.yaml"); assert_eq!(c.policies[1].path, "quality.yaml"); } + #[test] + fn parse_contract_all_pass_spelling_deserializes_as_worst_case() { + let yaml = r#" +name: alt-spelling +version: "1.0.0" +combine: all_pass +policies: + - path: p.yaml +"#; + let c = parse_contract_str(yaml).unwrap(); + assert_eq!(c.combine, crate::contract::CombineRule::WorstCase); + } + #[test] fn parse_contract_empty_policies_invalid() { let yaml = r#" @@ -115,6 +129,6 @@ policies: - path: single.yaml "#; let c = parse_contract_str(yaml).unwrap(); - assert_eq!(c.combine, crate::contract::CombineRule::AllPass); + assert_eq!(c.combine, crate::contract::CombineRule::WorstCase); } } diff --git a/geval/src/contract/mod.rs b/geval/src/contract/mod.rs index 8f09426..09843c6 100644 --- a/geval/src/contract/mod.rs +++ b/geval/src/contract/mod.rs @@ -1,7 +1,8 @@ //! Contract: a named, versioned set of policies evaluated together with a combination rule. //! //! This is the core unit of evaluation in Geval. A contract references multiple policy files; -//! each policy is evaluated against the same signals; outcomes are combined (e.g. all_pass, any_block_blocks) +//! each policy is evaluated against the same signals; outcomes are merged with **worst_case** +//! (BLOCK > REQUIRE_APPROVAL > PASS) //! into a single decision. mod combine; diff --git a/geval/src/contract/runner.rs b/geval/src/contract/runner.rs index ddb44bf..04b839b 100644 --- a/geval/src/contract/runner.rs +++ b/geval/src/contract/runner.rs @@ -22,9 +22,11 @@ pub struct PolicyResult { pub policy_version: Option, /// Outcome for this policy. pub outcome: DecisionOutcome, - /// Matched rule name (if any). + /// Winning rule name (if any): best priority among rules whose `when` matched. pub matched_rule: Option, - /// Reason from the matched rule (if any). + /// All rule names whose `when` matched, in priority order (1 first). + pub matching_rules: Vec, + /// Reason from the winning rule (if any). pub reason: Option, } @@ -79,6 +81,7 @@ pub fn run_contract( policy_version: policy.version.clone(), outcome: decision.outcome, matched_rule: decision.matched_rule.clone(), + matching_rules: decision.matching_rules.clone(), reason: decision.reason.clone(), }); } @@ -140,6 +143,7 @@ fn combined_decision_from_results( outcome: DecisionOutcome::Pass, matched_rule: None, reason: None, + matching_rules: Vec::new(), }; } let first_non_pass = results.iter().find(|r| r.outcome != DecisionOutcome::Pass); @@ -148,11 +152,13 @@ fn combined_decision_from_results( outcome, matched_rule: r.matched_rule.clone().map(|rule| format!("{}:{}", r.policy_path, rule)), reason: r.reason.clone(), + matching_rules: Vec::new(), }, None => Decision { outcome, matched_rule: None, reason: None, + matching_rules: Vec::new(), }, } } @@ -167,6 +173,7 @@ fn overall_decision_from_contracts( outcome: DecisionOutcome::Pass, matched_rule: None, reason: None, + matching_rules: Vec::new(), }; } let first_non_pass = entries @@ -182,12 +189,14 @@ fn overall_decision_from_contracts( outcome, matched_rule, reason: d.reason.clone(), + matching_rules: Vec::new(), } } None => Decision { outcome, matched_rule: None, reason: None, + matching_rules: Vec::new(), }, } } @@ -217,7 +226,7 @@ mod tests { let contract = ContractDef { name: "test".to_string(), version: "1.0".to_string(), - combine: CombineRule::AllPass, + combine: CombineRule::WorstCase, policies: vec![PolicyRef { path: "p.yaml".to_string(), }], @@ -241,15 +250,16 @@ rules: let result = run_contract(&contract, &[policy], &graph).unwrap(); assert_eq!(result.policy_results.len(), 1); assert_eq!(result.policy_results[0].outcome, DecisionOutcome::Pass); + assert!(result.policy_results[0].matching_rules.is_empty()); assert_eq!(result.combined_decision.outcome, DecisionOutcome::Pass); } #[test] - fn run_contract_two_policies_all_pass_combined_block() { + fn run_contract_two_policies_worst_case_combined_block() { let contract = ContractDef { name: "test".to_string(), version: "1.0".to_string(), - combine: CombineRule::AllPass, + combine: CombineRule::WorstCase, policies: vec![ PolicyRef { path: "a.yaml".to_string(), @@ -291,16 +301,18 @@ rules: let graph = SignalGraph::build(&signals.signals); let result = run_contract(&contract, &[policy_a, policy_b], &graph).unwrap(); assert_eq!(result.policy_results[0].outcome, DecisionOutcome::Pass); + assert_eq!(result.policy_results[0].matching_rules, vec!["pass"]); assert_eq!(result.policy_results[1].outcome, DecisionOutcome::Block); + assert_eq!(result.policy_results[1].matching_rules, vec!["block_low"]); assert_eq!(result.combined_decision.outcome, DecisionOutcome::Block); } #[test] - fn run_contract_any_block_blocks() { + fn run_contract_worst_case_one_policy_block_merges() { let contract = ContractDef { name: "test".to_string(), version: "1.0".to_string(), - combine: CombineRule::AnyBlockBlocks, + combine: CombineRule::WorstCase, policies: vec![ PolicyRef { path: "a.yaml".to_string(), @@ -333,7 +345,7 @@ rules: } #[test] - fn load_run_contracts_two_all_pass_overall_pass() { + fn load_run_contracts_two_contracts_overall_pass() { let dir = tempfile::tempdir().unwrap(); let p1 = dir.path().join("p1.yaml"); let p2 = dir.path().join("p2.yaml"); @@ -372,7 +384,7 @@ rules: r#" name: c1 version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: p1.yaml "#, @@ -383,7 +395,7 @@ policies: r#" name: c2 version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: p2.yaml "#, @@ -395,7 +407,7 @@ policies: let run = load_run_contracts( &[c1, c2], &graph, - CombineRule::AllPass, + CombineRule::WorstCase, ) .unwrap(); assert_eq!(run.entries.len(), 2); @@ -442,7 +454,7 @@ policies: r#" name: c1 version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: p1.yaml "#, @@ -453,7 +465,7 @@ policies: r#" name: c2 version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: p2.yaml "#, @@ -465,7 +477,7 @@ policies: let run = load_run_contracts( &[c1.clone(), c2], &graph, - CombineRule::AllPass, + CombineRule::WorstCase, ) .unwrap(); assert_eq!(run.overall.outcome, DecisionOutcome::Block); @@ -473,7 +485,7 @@ policies: } #[test] - fn load_run_contracts_any_block_blocks_across_contracts() { + fn load_run_contracts_worst_case_across_two_contracts() { let dir = tempfile::tempdir().unwrap(); let p1 = dir.path().join("p1.yaml"); let p2 = dir.path().join("p2.yaml"); @@ -494,7 +506,7 @@ policies: r#" name: c1 version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: p1.yaml "#, @@ -505,7 +517,7 @@ policies: r#" name: c2 version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: p2.yaml "#, @@ -514,7 +526,7 @@ policies: let signals = SignalSet::new(vec![sig(None, "x", 1.0), sig(None, "y", 1.0)]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&[c1, c2], &graph, CombineRule::AnyBlockBlocks).unwrap(); + let run = load_run_contracts(&[c1, c2], &graph, CombineRule::WorstCase).unwrap(); assert_eq!(run.overall.outcome, DecisionOutcome::Pass); let signals_block = SignalSet::new(vec![sig(None, "x", 20.0), sig(None, "y", 1.0)]); @@ -522,7 +534,7 @@ policies: let c1b = dir.path().join("contract1.yaml"); let c2b = dir.path().join("contract2.yaml"); let run_b = - load_run_contracts(&[c1b, c2b], &graph_b, CombineRule::AnyBlockBlocks).unwrap(); + load_run_contracts(&[c1b, c2b], &graph_b, CombineRule::WorstCase).unwrap(); assert_eq!(run_b.overall.outcome, DecisionOutcome::Block); } @@ -530,13 +542,13 @@ policies: fn load_run_contracts_empty_paths_errors() { let signals = SignalSet::new(vec![sig(None, "x", 1.0)]); let graph = SignalGraph::build(&signals.signals); - let err = load_run_contracts(&[], &graph, CombineRule::AllPass).unwrap_err(); + let err = load_run_contracts(&[], &graph, CombineRule::WorstCase).unwrap_err(); assert!(err.to_string().contains("at least one contract")); } - /// all_pass across contracts: PASS + REQUIRE_APPROVAL → overall REQUIRE_APPROVAL (no BLOCK). + /// worst_case across contracts: PASS + REQUIRE_APPROVAL → overall REQUIRE_APPROVAL (no BLOCK). #[test] - fn load_run_contracts_all_pass_pass_and_require_approval_overall_require_approval() { + fn load_run_contracts_pass_and_require_approval_overall_require_approval() { let dir = tempfile::tempdir().unwrap(); let p_ok = dir.path().join("ok.yaml"); let p_appr = dir.path().join("appr.yaml"); @@ -574,7 +586,7 @@ policies: &c1, r#"name: gate-a version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: ok.yaml "#, @@ -584,7 +596,7 @@ policies: &c2, r#"name: gate-b version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: appr.yaml "#, @@ -594,7 +606,7 @@ policies: let signals = SignalSet::new(vec![sig(None, "x", 1.0), sig(None, "z", 0.5)]); let graph = SignalGraph::build(&signals.signals); let c2p = c2.clone(); - let run = load_run_contracts(&[c1, c2], &graph, CombineRule::AllPass).unwrap(); + let run = load_run_contracts(&[c1, c2], &graph, CombineRule::WorstCase).unwrap(); assert_eq!(run.entries[0].result.combined_decision.outcome, DecisionOutcome::Pass); assert_eq!( run.entries[1].result.combined_decision.outcome, @@ -633,7 +645,7 @@ policies: &c1, r#"name: first version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: ok.yaml "#, @@ -643,7 +655,7 @@ policies: &c2, r#"name: second version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: blk.yaml "#, @@ -652,14 +664,14 @@ policies: let signals = SignalSet::new(vec![sig(None, "x", 1.0), sig(None, "w", 1.0)]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&[c1, c2.clone()], &graph, CombineRule::AllPass).unwrap(); + let run = load_run_contracts(&[c1, c2.clone()], &graph, CombineRule::WorstCase).unwrap(); assert_eq!(run.overall.outcome, DecisionOutcome::Block); assert!(run.overall.matched_rule.unwrap().contains(c2.to_str().unwrap())); } - /// BLOCK in first contract wins over REQUIRE_APPROVAL in second under all_pass. + /// BLOCK in first contract wins over REQUIRE_APPROVAL in second (worst_case merge). #[test] - fn load_run_contracts_all_pass_block_before_require_approval_second_contract() { + fn load_run_contracts_block_wins_over_require_approval_across_contracts() { let dir = tempfile::tempdir().unwrap(); let p_block = dir.path().join("blk.yaml"); let p_appr = dir.path().join("appr.yaml"); @@ -679,7 +691,7 @@ policies: &c1, r#"name: blocks-first version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: blk.yaml "#, @@ -689,7 +701,7 @@ policies: &c2, r#"name: appr-second version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: appr.yaml "#, @@ -698,14 +710,14 @@ policies: let signals = SignalSet::new(vec![sig(None, "x", 20.0), sig(None, "z", 0.1)]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&[c1.clone(), c2], &graph, CombineRule::AllPass).unwrap(); + let run = load_run_contracts(&[c1.clone(), c2], &graph, CombineRule::WorstCase).unwrap(); assert_eq!(run.overall.outcome, DecisionOutcome::Block); assert!(run.overall.matched_rule.unwrap().contains(c1.to_str().unwrap())); } - /// any_block_blocks: no BLOCK anywhere → PASS + REQUIRE_APPROVAL → REQUIRE_APPROVAL. + /// No BLOCK anywhere → PASS + REQUIRE_APPROVAL → overall REQUIRE_APPROVAL. #[test] - fn load_run_contracts_any_block_blocks_pass_and_require_approval() { + fn load_run_contracts_pass_and_require_approval_overall_when_no_block() { let dir = tempfile::tempdir().unwrap(); let p_ok = dir.path().join("ok.yaml"); let p_appr = dir.path().join("appr.yaml"); @@ -725,7 +737,7 @@ policies: &c1, r#"name: a version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: ok.yaml "#, @@ -735,7 +747,7 @@ policies: &c2, r#"name: b version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: appr.yaml "#, @@ -744,12 +756,12 @@ policies: let signals = SignalSet::new(vec![sig(None, "x", 1.0), sig(None, "z", 0.1)]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&[c1, c2], &graph, CombineRule::AnyBlockBlocks).unwrap(); + let run = load_run_contracts(&[c1, c2], &graph, CombineRule::WorstCase).unwrap(); assert_eq!(run.overall.outcome, DecisionOutcome::RequireApproval); } #[test] - fn load_run_contracts_three_contracts_all_pass() { + fn load_run_contracts_three_contracts_overall_pass() { let dir = tempfile::tempdir().unwrap(); for i in 1..=3 { let p = dir.path().join(format!("p{}.yaml", i)); @@ -779,7 +791,7 @@ policies: format!( r#"name: c{} version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: p{}.yaml "#, @@ -795,12 +807,12 @@ policies: sig(None, "m3", 1.0), ]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&paths, &graph, CombineRule::AllPass).unwrap(); + let run = load_run_contracts(&paths, &graph, CombineRule::WorstCase).unwrap(); assert_eq!(run.entries.len(), 3); assert_eq!(run.overall.outcome, DecisionOutcome::Pass); } - /// One contract with two policies (internal all_pass); partner contract passes — overall pass. + /// One contract with two policies (internal worst_case); partner contract passes — overall pass. #[test] fn load_run_contracts_partner_passes_when_first_has_two_policies_internal_combine() { let dir = tempfile::tempdir().unwrap(); @@ -828,7 +840,7 @@ policies: &c_multi, r#"name: dual-policy-gate version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: pa.yaml - path: pb.yaml @@ -839,7 +851,7 @@ policies: &c_single, r#"name: partner version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: partner.yaml "#, @@ -852,7 +864,7 @@ policies: sig(None, "z", 1.0), ]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&[c_multi, c_single], &graph, CombineRule::AllPass).unwrap(); + let run = load_run_contracts(&[c_multi, c_single], &graph, CombineRule::WorstCase).unwrap(); assert_eq!(run.entries[0].result.policy_results.len(), 2); assert_eq!(run.overall.outcome, DecisionOutcome::Pass); } @@ -873,7 +885,7 @@ policies: &c_first, r#"name: alpha version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: blk.yaml "#, @@ -883,7 +895,7 @@ policies: &c_second, r#"name: beta version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: blk.yaml "#, @@ -895,7 +907,7 @@ policies: let run_a_then_b = load_run_contracts( &[c_first.clone(), c_second.clone()], &graph, - CombineRule::AllPass, + CombineRule::WorstCase, ) .unwrap(); let rule_ab = run_a_then_b.overall.matched_rule.unwrap(); @@ -905,7 +917,7 @@ policies: rule_ab ); - let run_b_then_a = load_run_contracts(&[c_second, c_first], &graph, CombineRule::AllPass).unwrap(); + let run_b_then_a = load_run_contracts(&[c_second, c_first], &graph, CombineRule::WorstCase).unwrap(); let rule_ba = run_b_then_a.overall.matched_rule.unwrap(); assert!( rule_ba.contains("contract_beta"), diff --git a/geval/src/evaluator/engine.rs b/geval/src/evaluator/engine.rs index 69116ea..4e876eb 100644 --- a/geval/src/evaluator/engine.rs +++ b/geval/src/evaluator/engine.rs @@ -1,4 +1,6 @@ -//! Evaluation engine: for each rule in priority order, if rule matches signal graph then return that decision; else PASS. +//! Evaluation engine: every rule is checked against the signal graph; **all** matches are recorded, +//! and the **winning** rule is the one with the **best** priority (**1** = highest; larger numbers are lower). +//! If no rule matches, the policy outcome is PASS. use crate::policy::{Action, Operator, Policy, Rule}; use crate::signal_graph::SignalGraph; @@ -35,8 +37,12 @@ pub enum DecisionOutcome { #[derive(Debug, Clone, Serialize)] pub struct Decision { pub outcome: DecisionOutcome, + /// Winning rule name (best priority among those whose `when` matched). pub matched_rule: Option, pub reason: Option, + /// Names of all rules whose `when` matched, in **priority order** (ascending; 1 first). + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub matching_rules: Vec, } impl Decision { @@ -45,12 +51,13 @@ impl Decision { outcome: DecisionOutcome::Pass, matched_rule: None, reason: None, + matching_rules: Vec::new(), } } } -/// Evaluate policy against signal graph. Rules are evaluated in priority order; -/// first matching rule determines the decision. If no rule matches, return PASS. +/// Evaluate policy against the signal graph. Every rule is tested; the outcome comes from the +/// matching rule with the **smallest** `priority` number (**1** = highest precedence). pub fn evaluate(policy: &Policy, graph: &SignalGraph) -> Decision { let (decision, _) = evaluate_with_trace(policy, graph); decision @@ -59,6 +66,7 @@ pub fn evaluate(policy: &Policy, graph: &SignalGraph) -> Decision { /// Like evaluate, but also returns a trace of each rule evaluation for display. pub fn evaluate_with_trace(policy: &Policy, graph: &SignalGraph) -> (Decision, Vec) { let mut trace = Vec::new(); + let mut matching_rules = Vec::new(); for rule in policy.sorted_rules() { let (matched, condition, signal_value, _threshold) = rule_match_detail(rule, graph); trace.push(RuleTrace { @@ -75,22 +83,29 @@ pub fn evaluate_with_trace(policy: &Policy, graph: &SignalGraph) -> (Decision, V reason: rule.then.reason.clone(), }); if matched { - let outcome = match rule.then.action { - Action::Pass => DecisionOutcome::Pass, - Action::Block => DecisionOutcome::Block, - Action::RequireApproval => DecisionOutcome::RequireApproval, - }; - return ( - Decision { - outcome, - matched_rule: Some(rule.name.clone()), - reason: rule.then.reason.clone(), - }, - trace, - ); + matching_rules.push(rule.name.clone()); } } - (Decision::pass(), trace) + let winner = trace + .iter() + .filter(|t| t.matched) + .min_by_key(|t| t.priority); + let decision = if let Some(t) = winner { + let outcome = match t.action { + Action::Pass => DecisionOutcome::Pass, + Action::Block => DecisionOutcome::Block, + Action::RequireApproval => DecisionOutcome::RequireApproval, + }; + Decision { + outcome, + matched_rule: Some(t.rule_name.clone()), + reason: t.reason.clone(), + matching_rules, + } + } else { + Decision::pass() + }; + (decision, trace) } /// Returns (matched, condition_string, signal_value, threshold). @@ -218,10 +233,11 @@ rules: let d = evaluate(&policy, &graph); assert_eq!(d.outcome, DecisionOutcome::Pass); assert!(d.matched_rule.is_none()); + assert!(d.matching_rules.is_empty()); } #[test] - fn test_first_matching_rule_wins() { + fn test_best_priority_wins_when_multiple_rules_match() { let policy = parse_policy_str( r#" rules: @@ -252,9 +268,10 @@ rules: ]); let graph = SignalGraph::build(&signals.signals); let d = evaluate(&policy, &graph); - // Priority 1 matches first: hallucination_guard + // Priority 1 beats 2: both can match, hallucination wins assert_eq!(d.outcome, DecisionOutcome::Block); assert_eq!(d.matched_rule.as_deref(), Some("hallucination")); + assert_eq!(d.matching_rules, vec!["hallucination", "retrieval"]); } #[test] @@ -288,8 +305,43 @@ rules: ]); let graph = SignalGraph::build(&signals.signals); let d = evaluate(&policy, &graph); - // First rule matches: human_reviewed is present (even without a score). + // Priority 1 matches: human_reviewed is present (even without a score). assert_eq!(d.outcome, DecisionOutcome::RequireApproval); assert_eq!(d.matched_rule.as_deref(), Some("require_human_review")); + assert_eq!(d.matching_rules, vec!["require_human_review"]); + } + + #[test] + fn test_all_matches_recorded_best_priority_wins() { + let policy = parse_policy_str( + r#" +rules: + - priority: 5 + name: would_block + when: + metric: x + operator: ">" + threshold: 0 + then: + action: block + - priority: 2 + name: wins_pass + when: + metric: x + operator: ">" + threshold: 0 + then: + action: pass +"#, + ) + .unwrap(); + let signals = SignalSet::new(vec![sig(None, "x", 1.0)]); + let graph = SignalGraph::build(&signals.signals); + let (d, trace) = evaluate_with_trace(&policy, &graph); + assert_eq!(d.outcome, DecisionOutcome::Pass); + assert_eq!(d.matched_rule.as_deref(), Some("wins_pass")); + assert_eq!(d.matching_rules, vec!["wins_pass", "would_block"]); + let matched_names: Vec<_> = trace.iter().filter(|t| t.matched).map(|t| t.rule_name.as_str()).collect(); + assert_eq!(matched_names, vec!["wins_pass", "would_block"]); } } diff --git a/geval/src/explanation/explain.rs b/geval/src/explanation/explain.rs index 3d26ca9..b3637e5 100644 --- a/geval/src/explanation/explain.rs +++ b/geval/src/explanation/explain.rs @@ -25,11 +25,17 @@ pub fn explain_contract_result( } out.push_str("\nPer-policy results:\n"); for r in &result.policy_results { - let match_info = r - .matched_rule - .as_ref() - .map(|m| format!(" (matched: {})", m)) - .unwrap_or_default(); + let mut match_info = String::new(); + if !r.matching_rules.is_empty() { + let _ = write!( + match_info, + " (rules that matched: {})", + r.matching_rules.join(", ") + ); + } + if let Some(m) = &r.matched_rule { + let _ = write!(match_info, " [winner: {}]", m); + } let _ = writeln!( out, " {}: {}{}", @@ -83,11 +89,17 @@ pub fn explain_multi_contract_result( let _ = writeln!(out, "Combine rule (policies): {}", result.combine_rule); out.push_str("Per-policy results:\n"); for r in &result.policy_results { - let match_info = r - .matched_rule - .as_ref() - .map(|m| format!(" (matched: {})", m)) - .unwrap_or_default(); + let mut match_info = String::new(); + if !r.matching_rules.is_empty() { + let _ = write!( + match_info, + " (rules that matched: {})", + r.matching_rules.join(", ") + ); + } + if let Some(m) = &r.matched_rule { + let _ = write!(match_info, " [winner: {}]", m); + } let _ = writeln!( out, " {}: {}{}", @@ -141,11 +153,17 @@ pub fn explain_decision( let _ = writeln!(out, " {} = {}", label, value_str); } - out.push_str("\nMatched Rule:\n"); - if let Some(ref name) = decision.matched_rule { - let _ = writeln!(out, "{}", name); + if !decision.matching_rules.is_empty() { + out.push_str("\nRules that matched (priority order, 1 = highest):\n"); + for name in &decision.matching_rules { + let _ = writeln!(out, " {}", name); + } + out.push_str("Winning rule (best priority):\n"); + if let Some(ref name) = decision.matched_rule { + let _ = writeln!(out, " {}", name); + } } else { - out.push_str("(none — default PASS)\n"); + out.push_str("\nMatched rules:\n(none — default PASS)\n"); } out.push_str("\nDecision:\n"); @@ -232,7 +250,7 @@ mod tests { format!( r#"name: {} version: "1.0.0" -combine: all_pass +combine: worst_case policies: - path: p.yaml "#, @@ -243,12 +261,14 @@ policies: } let signals = SignalSet::new(vec![sig("x", 1.0)]); let graph = SignalGraph::build(&signals.signals); - let run = load_run_contracts(&[c1, c2], &graph, CombineRule::AllPass).unwrap(); + let run = load_run_contracts(&[c1, c2], &graph, CombineRule::WorstCase).unwrap(); let text = explain_multi_contract_result(&run, &graph, None); assert!(text.contains("MULTI-CONTRACT")); assert!(text.contains("Combine contracts rule")); assert!(text.contains("OVERALL (PR)")); assert!(text.contains("PASS")); assert!(text.contains("Contract file:")); + assert!(text.contains("rules that matched")); + assert!(text.contains("[winner:")); } } diff --git a/geval/src/policy/model.rs b/geval/src/policy/model.rs index 9471e0a..40c45b6 100644 --- a/geval/src/policy/model.rs +++ b/geval/src/policy/model.rs @@ -78,7 +78,8 @@ pub struct RuleConsequence { pub reason: Option, } -/// A single policy rule: priority (lower = evaluated first), name, when, then. +/// A single policy rule: **priority** (**1** = highest precedence; larger numbers are lower), name, when, then. +/// Priorities must be **unique** within a policy (enforced when loading YAML). #[derive(Debug, Clone, Deserialize, Serialize)] pub struct Rule { pub priority: u32, @@ -103,7 +104,7 @@ pub struct Policy { } impl Policy { - /// Rules sorted by priority (ascending); first match wins. + /// Rules sorted by priority (ascending) for stable evaluation order. pub fn sorted_rules(&self) -> Vec<&Rule> { let mut r: Vec<&Rule> = self.rules.iter().collect(); r.sort_by_key(|x| x.priority); diff --git a/geval/src/policy/parser.rs b/geval/src/policy/parser.rs index 69a90f4..dfcf834 100644 --- a/geval/src/policy/parser.rs +++ b/geval/src/policy/parser.rs @@ -32,30 +32,44 @@ struct PolicyInner { rules: Option>, } +fn validate_unique_rule_priorities(policy: &Policy) -> Result<()> { + let mut seen = std::collections::HashSet::new(); + for rule in &policy.rules { + if !seen.insert(rule.priority) { + anyhow::bail!( + "duplicate rule priority {} in policy: each rule must have a unique priority (1 = highest precedence)", + rule.priority + ); + } + } + Ok(()) +} + fn parse_policy_yaml(s: &str) -> Result { let _: serde_yaml::Value = serde_yaml::from_str(s).context("parse policy YAML")?; let wrapped: Option = serde_yaml::from_str(s).ok(); - if let Some(f) = wrapped { + let policy = if let Some(f) = wrapped { if let Some(inner) = f.policy { - return Ok(Policy { + Policy { name: inner.name.or(f.name), version: inner.version.or(f.version), environment: inner.environment.or(f.environment), rules: inner.rules.unwrap_or_else(Vec::new), - }); + } + } else { + Policy { + name: f.name, + version: f.version, + environment: f.environment, + rules: f.rules.unwrap_or_else(Vec::new), + } } - return Ok(Policy { - name: f.name, - version: f.version, - environment: f.environment, - rules: f.rules.unwrap_or_else(Vec::new), - }); - } - - // Direct policy shape - let p: Policy = serde_yaml::from_str(s).context("invalid policy structure")?; - Ok(p) + } else { + serde_yaml::from_str(s).context("invalid policy structure")? + }; + validate_unique_rule_priorities(&policy)?; + Ok(policy) } /// Parse policy from a string (e.g. for tests or inline). @@ -120,4 +134,30 @@ policy: assert_eq!(p.version.as_deref(), Some("2.1.0")); assert_eq!(p.rules.len(), 1); } + + #[test] + fn duplicate_priority_rejected() { + let yaml = r#" +rules: + - priority: 1 + name: a + when: + metric: x + operator: ">" + threshold: 0 + then: + action: pass + - priority: 1 + name: b + when: + metric: y + operator: ">" + threshold: 0 + then: + action: block +"#; + let err = parse_policy_str(yaml).unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("duplicate rule priority"), "{}", msg); + } } diff --git a/geval/src/reconciliation/rules.rs b/geval/src/reconciliation/rules.rs index a28a920..0ea104d 100644 --- a/geval/src/reconciliation/rules.rs +++ b/geval/src/reconciliation/rules.rs @@ -1,8 +1,6 @@ //! Reconciliation via priority rules only. No scoring or weights. -//! The evaluator already evaluates rules in priority order; this module -//! is a placeholder for any explicit reconciliation documentation or -//! future extension (e.g. named reconciliation strategies). -//! Actual behaviour: first matching rule wins; no match => PASS. +//! The evaluator tests every rule, records all matches, and picks the **best priority** +//! (**1** = highest); no match => PASS. This module is a placeholder for future extensions. use crate::evaluator::Decision;