Repository navigation
Broaden the Critic to every detector; share the seam via ephor core (ADR-0023) - #76
Merged
Merged
Conversation
…ADR-0023) Critique, the Critic Protocol, and FakeCritic move out of cloud_waste/critic.py into ephor.critic - a shared core primitive needing nothing beyond stdlib/ dataclasses. ClaudeCritic (the real, paid implementation) stays in cloud-waste deliberately, so ephor itself never gains anthropic as a dependency. stripe_recovery.detector.scan_for_recoverable_charges and wallet_guard.detector.scan_for_risky_approvals both gain the same optional critic: Critic | None = None param cloud-waste's scans already had, attaching a critique to the Snapshot before it's hashed when one is passed. Zero behaviour change for any existing caller. Both demos now wire in FakeCritic() and print the critique line, same as cloud-waste's demo already does. ADR-0021 scoped the Critic to cloud-waste specifically, since idle-instance detection was the actual judgement call it solved - that reasoning argued the Critic wasn't necessary elsewhere, not that it would be harmful. An advisory- only critique (never a veto, either way) costs nothing safety-wise even on an already-unambiguous rule, and can still flag an evidence-level anomaly the fixed rule never looks at. Written up as ADR-0023, filed and closes #75. All four packages verified independently: ruff/mypy clean, full pytest green (ephor, cloud-waste 51 tests, stripe-recovery 32, wallet-guard 25), all three demos run end to end with the critic line printing correctly. Closes #75
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
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.
Closes #75.
What
Critique, theCriticProtocol, andFakeCriticmove out ofcloud_waste/critic.pyintoephor.critic- a shared core primitive needing nothing beyond stdlib/dataclasses, zero new dependencies forephor.ClaudeCritic/ClaudeCriticSettings(the real, paid implementation) stay incloud_waste/critic.py, importingCritique/Criticfromephor.critic- keepsanthropicout ofephor's own dependency list entirely.stripe_recovery.detector.scan_for_recoverable_chargesandwallet_guard.detector.scan_for_risky_approvalsboth gain the same optionalcritic: Critic | None = Noneparamcloud-waste's two scans already have, attachingcritique.as_snapshot_field()to the Snapshot before it's hashed when one is passed. Zero behaviour change for any existing caller. Both demos now wire inFakeCritic()and print the critique line, same ascloud-waste's demo already does - no realClaudeCriticoption is wired into either, that stayscloud-waste-only.Why extend the seam, given ADR-0021 scoped the Critic to cloud-waste specifically
ADR-0021 reasoned the Critic belongs on
cloud-wastebecause idle-instance detection is a genuine judgement call, unlikestripe-recovery/wallet-guard's already-unambiguous allow-list checks. That argued the Critic wasn't necessary elsewhere - not that it would be harmful. An advisory-only critique (never a veto, per ADR-0021 point 3) costs nothing safety-wise even on an already-unambiguous rule, and can still flag an evidence-level anomaly the fixed rule was never written to look at. Written up as ADR-0023.Verification
All four packages verified independently:
ephor:ruff check/ruff format --check/mypy .clean,pytest -vgreen (newephor/tests/test_critic.py).cloud-waste: same, 51 tests passing (down from 54 - threeFakeCritic/Critiquetests moved toephor, not lost).stripe-recovery: same, 32 tests passing.wallet-guard: same, 25 tests passing.uv run python -m <pkg>.demo) run end to end with the critic line printing correctly.ephor/,cloud-waste/,stripe-recovery/,wallet-guard/, anddocs/adr/- nobackend//frontend/changes.