refactor(playbook): drop the provisional confirmation seam - #505
Merged
Merged
Conversation
The lifecycle termination triple is a pair. `restore` and `displace` remain; `confirm_user_playbook_provisional_publication` is deleted from both the shared `UserPlaybookLifecycleTerminationStore` Protocol and the storage base's default-raising stub. It was never callable. The transition required a confirmation analyst that is a Protocol with no implementation, and `QualificationCase` refuses `role="confirmation"` at parse time, so the support tally the boundary compared against its two-session floor was permanently zero. Every publication therefore reverted at its deadline regardless of quality. The enterprise adapter's implementation and its only caller are already gone. A published revision is now KEPT until the tuner's own next evidence pass revises it, so nothing can ask for the transition -- and a declared-but-uncallable Protocol member obliges every backend to implement one anyway. The SQL routine is untouched: dropping it is a contract migration that needs its own rollout sequencing.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe playbook lifecycle termination interfaces no longer declare provisional publication confirmation. Restore and displace operations remain available. The user playbook store documentation reflects the reduced interface. ChangesPlaybook termination interfaces
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to The removed confirmation transition has no current dependency, so this interface cleanup is ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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.
The open-source half of the enterprise keep-unless-revised change
(ReflexioAI/reflexio-enterprise#1157, merged as
98686f2f0). The enterprisegitlink on
mainalready points at this commit, so landing this is what movesthat pointer off a feature branch.
What this removes
confirm_user_playbook_provisional_publication, from the storage protocol andthe
storage_basedeclaration. 2 files, +12 / −49.It was one third of a termination triple over the same lifecycle row —
restore,displace,confirm— declared here rather than as anenterprise-only extra because the shared Protocol
(
UserPlaybookLifecycleTerminationStore) lives in this package.Why it is safe to delete rather than leave declared
The enterprise side removed its implementation and its only caller, and it
did so because the transition was unreachable, not merely unused:
confirmwas reached only throughOpenWorldConfirmationBoundary, whosedecision required a
ConfirmationSessionAnalyst.slot, and
QualificationCaserejectedrole="confirmation"at parse time — sono qualification case could be authored for the role at all.
ConfirmationTallytherefore always reported zero supporting sessions againsta required minimum of two.
0 >= 2is false for every input.So the enterprise design replaced "revert unless confirmed after 14 days" with
"keep until the next evidence pass revises it". Publication is now written
terminal at publish time; nothing confirms, and nothing reverts automatically.
A declaration for a transition that can never be taken is a surface that reads
as a capability.
What stays
restoreanddisplaceare untouched. The two-thirds Protocol that remains isthe deliberate shape, and the comment replacing the deleted member says so — a
future reader finding a termination pair rather than a triple should find the
reason in place rather than infer an omission.
Verification
Carried on the enterprise branch that merged as #1157:
pytest reflexio_ext/tests -m "not integration"import reflexioThe three failures are pre-existing and proven so: two are migration-inventory
gates red on enterprise
mainbecause of an unrelated workstream's unownedmigration (
20260910100000_durable_sliding_extraction.sql), and the third is anunrelated deploy-topology test.
Summary by CodeRabbit