[XxFran10xX]

Why

On 2026-10-21 Main froze for 20–30 s once an hour from 16:57 until the 06:00 restart (14 Paper watchdog dumps). At 16:57, 13 held guildless estates were all decided at once.

• Estates stay decided for good. Estates.decided() only grows.
• The hourly pass settled every one again. CharacterGearEstate.reconcile (every 72000 ticks) settled every decided estate each hour, including ones Magic had already answered DONE.
• Each settle is expensive. It reads gear-stations.yml about five times on the main thread (requireReady → migrate, retained, loadViews, …).
• Why it was so slow on 0.9.0. The file was 5.4 MB, and every row was copied, so 13 estates cost 20–30 s.

#66 (in 0.11.0+) removed the per-row copy and archives released rows at startup, so today the pass is cheap. The cost still grows with the number of decided estates and with the station rows added during the day: each released row is about 120 KB and stays in the file until the next restart.

What changes

The hourly pass skips a revision that this process already reported DONE.

Case	Before	After
Revision already reported DONE	settled again every hour	skipped until the next restart
RETRY or BLOCKED answer	settled every hour	settled every hour (unchanged)
New revision of a decided estate	settled	settled (unchanged)
First pass after a restart	settles every decided estate	settles every decided estate (unchanged, reported is empty)
Settles called by RPCharacters at decision time	unchanged	unchanged

Trade-off: gear that reaches an already-settled testator later now moves at the first pass after the next daily restart, not within the hour. One example is a station that staff recover into a return. A dead character cannot deposit, so this should be rare. It is noted in the code.

Tests

• New hourlyReconcileSkipsARevisionAlreadyReportedDoneButSettlesRetriesAndNewRevisions covers four cases:
  • RETRY settles again every pass;
  • DONE is skipped;
  • a new revision settles once;
  • a fresh participant (a restart) settles again, and the guard still applies.
• mvn verify locally: 710 tests pass and CharacterGearEstate is at 100 % coverage. On Windows the run excludes the four known Windows-only failures (two case-spelling tests and two symlink tests, which also fail on main) and uses a local-only directory-fsync workaround that is not committed.
• Dev: see the test comment below.

Draft: not for merge or release yet.

🤖 Generated with Claude Code

[coderabbitai]

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: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 71f83f09-6837-4df0-a21d-301cdb34d656

📥 Commits

Reviewing files that changed from the base of the PR and between 32861d4 and 9cd61cf.

📒 Files selected for processing: 2

• src/main/java/net/tfminecraft/magic/gear/ownership/CharacterGearEstate.java
• src/test/java/net/tfminecraft/magic/gear/ownership/CharacterGearEstateTest.java

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

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Prevented estate decisions already completed for the same death and revision from being settled and reported again during subsequent runs.
  • Preserved retries when settlement effects are still pending, and ensured new revisions continue to be processed.
  • Added checks to ensure estate settlement applies to the matching death, including when a character’s death record changes.

Reconciliation now skips settlement when this process has already reported Outcome.Done for the same death ID and revision. Tests cover retry behaviour, new revisions and changes to the character’s death ID.

Changes: Estate reconciliation

Layer / File(s)	Summary
Skip previously completed decisions src/main/java/net/tfminecraft/magic/gear/ownership/CharacterGearEstate.java, src/test/java/net/tfminecraft/magic/gear/ownership/CharacterGearEstateTest.java	reconcile skips a decision only when the recorded result is Outcome.Done for the same death ID and revision. Tests cover held effects, released effects, new revisions and mismatched death IDs.

Priority: ➖ Normal

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

Change: Bug fix

Possibly related PRs

• TF-Minecraft/Magic#66: Both PRs change work on the estate and station-store read path, but use different mechanisms.

Suggested labels: size:M

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 9cd61

Repeated hourly passes skip decisions already reported DONE while retries and new revisions remain eligible. No actionable merge-blocking risk was identified.

- [ ] 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[XxFran10xX]

Dev test (TFMCDev01, 2026-10-22)

Jar magic-DEV-20261022-estate.jar (this branch) replaced magic-0.12.0.jar on Dev. Dev has 7 decided estates, and Magic had already answered DONE for all of them.

I recorded every file read with JFR (jdk.FileRead, threshold 0, with stacks) around both estate passes of a single uptime. Magic was enabled at 09:40:08 UTC, and Dev was not restarted between the two passes.

Pass	gear-stations.yml reads	from CharacterGearEstate.settle	from reconcile itself (migrate + loadViews)	estate journal reads in settle
First pass after restart (09:40:12)	82	70 (7 estates × 10)	4	56
Hourly pass (10:40:15)	4	0	4	0

• The first pass still settles every decided estate once, as intended.
• The hourly pass now skips the estates it already reported DONE.
• TPS stayed at 20.0 throughout, with no watchdog dumps.
• The only Magic log line was the existing Shrine cerrith.flowers unknown tag 'tall_flowers' warning, which is also in Dev's logs from before this jar.

[XxFran10xX]

Sol (gpt-6.1-sol) review: VERDICT: APPROVE, no blocking findings.

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.