Skip to content

[claude] Test that deleting an entity cascades through referrers - #151

Merged
myieye merged 1 commit into
mainfrom
claude/test-cascade-delete-97
Oct 23, 2026
Merged

myieye merged 1 commit into
mainfrom
claude/test-cascade-delete-97

Conversation

@myieye

@myieye myieye commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

[Claude, autonomous]

Adds a SnapshotWorkerTests case for the recursive MarkDeleted (word → definition → example in one delete commit); it fails if the recursion is removed.

Closes #97: most of it was already covered when it was filed. DataModelReferenceTests (DeleteRemovesAllReferences and the delete/reference block around it) and SnapshotWorkerTests.DeletingAnEntityDropsReferencesToItFromBaselineEntities cover reference removal. The cascade was only covered by ExampleSentenceTests.DeletingAWordDeletesTheExampleSentence, which only asserts that no entity is left undeleted. The loop guard still can't be tested: no sample model has a reference cycle where removing a reference deletes the referrer.


Generated by Claude Code

Covers the recursive MarkDeleted call in SnapshotWorker: deleting a word
deletes its definition via RemoveReference, which must in turn delete the
definition's example.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017iyZdteks1z5KpAsAAVMNt
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5b6fb925-186f-42d2-938d-2765a15cbe1c
📥 Commits

Reviewing files that changed from the base of the PR and between 5ef5d7e and 9cba179.

📒 Files selected for processing: 1
  • src/SIL.Harmony.Tests/SnapshotWorkerTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

Adds a test for recursive deletion. The test checks that deleting a Word marks the referencing Definition and Example as deleted and assigns all three snapshots the delete commit ID.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 9cba1

The focused regression test is valid and introduces no established merge-blocking risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title says the PR adds a test that deleting an entity cascades through its referrers. The raw summary and objectives confirm the change is a SnapshotWorkerTests case where deleting a Word recursiv…
Description check Passed The description states the PR adds a SnapshotWorkerTests case for recursive MarkDeleted (word to definition to example in one delete commit). It also explains the existing coverage and the untested lo…
Linked Issues check Passed Issue #97 requires coverage for reference removal and recursive cascade deletion. The existing SnapshotWorkerTests.DeletingAnEntityDropsReferencesToItFromBaselineEntities verifies reference scrubbin…
Out of Scope Changes check Passed The reviewed change adds one SnapshotWorkerTests case for the recursive MarkDeleted behavior required by issue #97. The change does not add unrelated production behavior or unrelated test coverage…
Full details: Title check

Explanation

The title says the PR adds a test that deleting an entity cascades through its referrers. The raw summary and objectives confirm the change is a SnapshotWorkerTests case where deleting a Word recursively deletes its Definition and Example. The title names the main change, so it passes. The '[claude]' prefix is an author tag and does not affect the assessment.

Full details: Description check

Explanation

The description states the PR adds a SnapshotWorkerTests case for recursive MarkDeleted (word to definition to example in one delete commit). It also explains the existing coverage and the untested loop guard, and it references issue #97. It is directly related to the changeset, so it passes.

Full details: Linked Issues check

Explanation

Issue #97 requires coverage for reference removal and recursive cascade deletion. The existing SnapshotWorkerTests.DeletingAnEntityDropsReferencesToItFromBaselineEntities verifies reference scrubbing. The new DeletingAnEntityCascadesThroughReferrersThatRemovingTheReferenceDeletes verifies that deleting a Word marks the Definition and Example deleted in the same commit. The test also verifies all three delete commit IDs. The issue notes that the recursion guard remains untested; it does not require a loop-specific test.

Full details: Out of Scope Changes check

Explanation

The reviewed change adds one SnapshotWorkerTests case for the recursive MarkDeleted behavior required by issue #97. The change does not add unrelated production behavior or unrelated test coverage.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@myieye
myieye merged commit c00599b into main Oct 23, 2026
7 checks passed
@myieye
myieye deleted the claude/test-cascade-delete-97 branch October 23, 2026 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test coverage: SnapshotWorker reference removal and cascade delete

2 participants