Show last-day and lifetime totals in the ledger view, and track deposits - #68
Merged
Merged
Conversation
Each ledger view icon (Citizens, Guild Taxes, Vassals, Tributes, Tariffs) now lists today's payers, what they paid on the last day, and what they have paid in total. A new Deposits icon does the same for members' /f deposit and /g deposit amounts. - Snapshot each capital's per-payer receipts from the payer side just before settlement, mirroring what applySettlementFor moves - Roll the snapshot into last-day and lifetime totals after settlement - Deposits are tracked only; settlement never reads the history - Save the history with the guild so a restart keeps it - Tributes "Today" uses the same internal income base as settlement Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
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: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds guild ledger history for daily and lifetime amounts, tracks current-day deposits, saves and restores this data, and displays the totals and contributors in guild ledger inventories. ChangesGuild Ledger History
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant FactionManager
participant Ledger
participant LedgerHistory
FactionManager->>Ledger: collectHistoryDay(getAllGuilds())
Ledger-->>FactionManager: per-guild history snapshots
FactionManager->>LedgerHistory: closeDay(snapshot) for each guild ledger
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable issue remains from the reviewed ledger history, deposit, or display changes. The PR is mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Players can see newly recorded deposit amounts and historical payer totals for factions they do not belong to. The history can also become inaccurate if a daily settlement step fails. The reviewed changes do not show a new way to move bank funds without authorization. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
A rabbit counts the guild’s deposits, Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
2 participants
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.
What
The faction ledger view icons (Citizens, Guild Taxes, Vassals, Tributes, Tariffs) now show three sections, each with a total and the top 5 payers:
A new Deposits icon (chest, slot 15) tracks members'
/f depositand/g depositamounts the same way. Deposits are recorded only. They are not income, and settlement never reads them.How
LedgerHistory(per guild ledger) holds last-day and lifetime maps per source, keyed by payer name, plus today's deposits.Ledger.collectHistoryDayruns once insettleIncomebefore Phase 1 clears citizen taxes. It walks payers the same wayapplySettlementFordoes (skipping bankrupt or bankless payers and receivers with no bank), so the snapshot matches what moves.ledger last day,ledger lifetime,deposits today). Old files load with empty history.getInternalTaxableIncome, the base settlement charges tribute on, instead of the gross base, so Today and Last day agree.Tests
LedgerHistoryTestcovers day rollover, lifetime accumulation, deposits, JSON round trip, old JSON, and the settlement snapshot (citizens, tariffs, bankrupt payer). Full suite passes locally (2298 tests).🤖 Generated with Claude Code
Summary by CodeRabbit