feat: add DecisionAssure Impact – counterfactual governance impact analysis - #3883
feat: add DecisionAssure Impact – counterfactual governance impact analysis#3883AkhileshWarik (a1k7) wants to merge 3 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
Imran Siddique (imran-siddique)
left a comment
There was a problem hiding this comment.
This addresses every structural point I raised on #3851, and it did so within the hour. Worth
recording what changed, because the difference is not cosmetic:
| #3851 | this PR | |
|---|---|---|
| size | 8,742 lines / 116 files | 1,346 lines / 35 files |
| mergeable | CONFLICTING (446 behind) | MERGEABLE |
| root-level scratch files | policy_v4/5/6.yaml, sample_traces.jsonl |
none |
| new top-level directories | decisionassure_continuity/, decisionassure_impact/ |
none |
| existing files modified | 7, including two SDK __init__.py |
zero |
agentmesh/governance.py vs the governance/ package |
collision | gone |
Landing it as agent-governance-python/agent-decisionassure/ with the samples under
examples/decisionassure/ is the right placement, and making the whole PR pure additions means it
cannot regress anything that exists. That is a much easier thing to approve.
Two other things you got right that I want to name. Per-package LICENSE, SECURITY.md and
CONTRIBUTING.md match what agent-os and its siblings carry, so this is not inventing a layout.
And docs/limitations.md is the most honest paragraph in the PR: "Replay establishes only what the
supplied artifacts support... Hashes detect integrity changes, not identity." A counterfactual engine
that oversold itself would be worse than none, and that page inoculates against it.
Reclaiming ADR 0033 is also correct rather than opportunistic, since #3121 was your own and was
closed unmerged. Incidentally that closes a gap: #3726 is currently numbered 0034 and leaves 0033
empty, so if both land the sequence is continuous.
One blocking problem: the test suite cannot run. The package was renamed during the re-cut and
the tests still import the old name. All four files fail at collection:
PYTHONPATH=src python -m pytest tests
tests/unit/test_policy.py:1
from decisionassure_impact.models import Action, Decision, Policy
E ModuleNotFoundError: No module named 'decisionassure_impact'
4 errors during collection (integration/test_replay.py, security/test_redaction.py,
unit/test_authority_evidence.py, unit/test_policy.py)
src/agent_decisionassure/ contains __init__.py, cli.py, drift.py, engine.py,
integration.py and models/. Every decisionassure_impact import needs to become
agent_decisionassure.
tests/security/test_redaction.py needs more than a rename. It imports
decisionassure_impact.security.redaction.redact, and there is no security/ package in this PR at
all. So either the redaction module was left behind in the re-cut, or that test is a leftover that
should be dropped. Given the file lives under tests/security/ and SECURITY.md promises that
credentials and raw customer evidence never reach traces or reports, I would rather see the module
arrive than the test disappear. A redact() that the docs commit to and the code does not implement
is the one gap here that is a correctness claim rather than a packaging detail.
Test coverage is thin for the size, and this is the natural moment to fix it since you are
touching all four files anyway. Roughly 37 lines of tests against 800+ lines of engine, CLI and drift
logic. The engine is the interesting part: a replay that flips a decision from ADMISSIBLE to
INADMISSIBLE is the product, so at least one end-to-end case per direction, driven from
examples/decisionassure/policy_v4.yaml to policy_v5.yaml over sample_traces.jsonl, would prove
the thing the PR exists to do.
None of that is a reason to re-cut again. Fix the imports, decide what happens to redaction, and this
becomes a straightforward review of the engine itself, which is where the attention belongs.
Thanks for turning this around so quickly and for taking the placement question seriously rather than
arguing it.
|
Thank you for the thorough review and the kind words about the structural improvements. I’ve addressed all the points you raised: Test imports – All tests now import from agent_decisionassure (the renamed package). Let me know if there’s anything else I can adjust. |
Problem & Solution
Problem:
AGT enforces policies at runtime, but there is no way to know what will break before you tighten a policy, expire a delegation, update a model, or expand a tool capability. Previously allowed actions may become inadmissible – but nobody knows which until after deployment.
Solution:
This PR introduces DecisionAssure Impact – a counterfactual governance replay engine that:
ADMISSIBLEtoINADMISSIBLE(and vice versa).ALLOW,REVIEW, orBLOCK.Changes
agent-governance-python/agent-decisionassure/package.examples/decisionassure/(synthetic generator, policy YAMLs).decisionassurewithimpactanddetect-driftsubcommands.agentmeshoragent_osimports.decisionassure impactexits with code 1 onBLOCK.Demo output