fix: preserve family invariants during parent replacement - #42
flyingrobots wants to merge 1 commit into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used🪛 ast-grep (0.45.3)test/family-model.py[info] 18-18: use secrets package over random package (avoid-random-python) [error] 25-25: Command coming from incoming request (subprocess-from-request) [error] 31-33: Avoid command injection (command-injection-python) [error] 31-33: Command coming from incoming request (subprocess-from-request) 🪛 Ruff (0.16.5)test/family-model.py[error] 19-19: Standard pseudo-random generators are not suitable for cryptographic purposes (S311) [warning] 25-25: Missing type annotation for (ANN002) [error] 26-26: (S603) [warning] 26-26: Add explicit (PLW1510) [warning] 26-26: Function definition does not bind loop variable (B023) [error] 32-32: (S603) [error] 33-33: Starting a process with a partial executable path (S607) 🔇 Additional comments (9)
📝 SummarySummary by CodeRabbit
WalkthroughChangesFamily replacement admission
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant FirstClaim
participant SecondClaim
participant plan_family
participant GitRefs
FirstClaim->>GitRefs: prepare parent or child admission
SecondClaim->>GitRefs: commit competing admission
FirstClaim->>plan_family: re-plan family observations
plan_family->>GitRefs: verify ancestor and descendant refs
GitRefs-->>plan_family: return current family state
plan_family-->>FirstClaim: accept or refuse with cycle/descendants
Merge Risk: ⚪ Minimal · up to Parent replacement now rejects cyclic or descendant-invalid transitions while preserving supported renewal and leaf replacement behavior; the supplied coverage indicates the change is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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. A rabbit checks each parent’s thread Comment |
Ordinary claims could create a parent cycle or replace Alice's parent acquisition with Bob's while leaving Alice's child attached. Admission now refuses parent replacement whenever stored descendants remain, including same-holder replacements and expired unswept descendants.
extendrenews the existing family; release or sweep ends the relevant descendants before replacement. Leaves can still be replaced or reparented.The README makes the policy explicit: child records name a parent job, but belong to the acquisition that admitted them. A batch may replace a leaf and then admit new children; it cannot admit a child and then replace its parent, or replace a parent whose children are already stored. Cycle checks include earlier batch plans. Observed ancestor records participate in the transaction's expected values. The JSON schema adds
parentrefusal detailscycleanddescendants, and the generated executable is rebuilt.Validation on
41e511e5936b57f72cfbb0822a598ddd4df55b1a:1701, operation39, where admission incorrectly returned 0.make testpassed 503 assertions, including 51 focused family assertions.doctorchecks 192 command operations across seeds34,1701and20260922, including full public family state, acquisition identities and unchanged refs after refusals.Tests ran with Bash 5.3.9 and Git 2.54.0. The suite changes
HOME, so the existingjsonschemainstallation was made available throughPYTHONPATH; no dependency was installed. An earlier test run hit host-disk exhaustion. The unchanged commit passed the full gate after space was recovered, using a dedicated temporary directory.The forced schedules test changes between observation and commit. They do not prove coherent membership/generation observation during partial visibility of a multi-ref transaction; #38 remains a separate investigation.
Fixes #34.