fix(ledger.lic): v1.4.2 track Account.name for better loot_cap estima…#2318
fix(ledger.lic): v1.4.2 track Account.name for better loot_cap estima…#2318mrhoribu wants to merge 2 commits into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughVersion 1.4.2 extends the ledger script with account-wide loot-cap estimation. A new AccountInfo module resolves account names, the database schema adds an account column and index for persistence, estimate_loot_cap queries now filter by account or fall back to character when unresolvable, transaction recording stores the account name, and chart rendering uses a simpler bar character. ChangesAccount-wide Ledger Tracking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/ledger.lic`:
- Around line 418-430: AccountInfo.name currently re-runs
Lich::Util.quiet_command_xml each time when Account.name is empty; memoize the
lookup (including the negative result) so the verb is sent at most once per
session: add a class-level cache (e.g., `@cached_account_name` and
`@cached_account_name_set`) used by AccountInfo.name to return a cached value if
set, only call Lich::Util.quiet_command_xml when the cache is unset, store
either the found name or nil into `@cached_account_name` and mark
`@cached_account_name_set`=true, and add an optional invalidation method (e.g.,
AccountInfo.invalidate_name_cache) so callers like record_transaction and
estimate_loot_cap will use the cached outcome and avoid repeated account
commands.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| def self.name | ||
| return nil unless Object.const_defined?(:Account) | ||
| return Account.name if Account.name && !Account.name.to_s.empty? | ||
|
|
||
| begin | ||
| Lich::Util.quiet_command_xml("account", /Account Name:/i, /<prompt/, false, 3) | ||
| rescue StandardError => e | ||
| echo "AccountInfo: failed to resolve via ACCOUNT (#{e.message})" | ||
| return nil | ||
| end | ||
|
|
||
| Account.name if Account.name && !Account.name.to_s.empty? | ||
| end |
There was a problem hiding this comment.
Memoize the unresolvable case to avoid repeated ACCOUNT commands on every transaction.
When Account.name is empty (e.g., proxy-mode logins, which the changelog explicitly calls out), this method falls through to quiet_command_xml("account", ...) and waits up to 3 seconds. Because record_transaction (line 611) calls AccountInfo.name on every recorded transaction, an unresolvable account will:
- Block the main
History.mainloop for ~3s per recorded transaction, risking missed/late deposit/withdraw matches that immediately follow. - Spam the in-game
accountcommand (visible server-side) every time silver/bounty changes.
The same applies to the account: default on estimate_loot_cap (line 348), though that path is colder.
Cache the resolution outcome (including the negative case) so the verb is sent at most once per session, with an optional invalidation hook if needed.
🔒 Proposed fix: memoize the resolved/unresolvable state
module AccountInfo
+ `@resolved` = false
+ `@cached_name` = nil
+
+ # Reset the cached resolution; useful if the user runs ;account later
+ # in the session and we want to retry.
+ def self.reset!
+ `@resolved` = false
+ `@cached_name` = nil
+ end
+
# Returns the current account name, populating it via the ACCOUNT verb if
# needed. Lich5 (Infomon) parses the ACCOUNT response and assigns
# Account.name itself; we just trigger the round-trip and read it back.
#
# `@return` [String, nil] account name, or nil if it cannot be resolved
def self.name
+ return `@cached_name` if `@resolved`
return nil unless Object.const_defined?(:Account)
- return Account.name if Account.name && !Account.name.to_s.empty?
+ if Account.name && !Account.name.to_s.empty?
+ `@resolved` = true
+ return `@cached_name` = Account.name
+ end
begin
Lich::Util.quiet_command_xml("account", /Account Name:/i, /<prompt/, false, 3)
rescue StandardError => e
echo "AccountInfo: failed to resolve via ACCOUNT (#{e.message})"
- return nil
+ `@resolved` = true
+ return `@cached_name` = nil
end
- Account.name if Account.name && !Account.name.to_s.empty?
+ `@resolved` = true
+ `@cached_name` = (Account.name if Account.name && !Account.name.to_s.empty?)
end
end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/ledger.lic` around lines 418 - 430, AccountInfo.name currently
re-runs Lich::Util.quiet_command_xml each time when Account.name is empty;
memoize the lookup (including the negative result) so the verb is sent at most
once per session: add a class-level cache (e.g., `@cached_account_name` and
`@cached_account_name_set`) used by AccountInfo.name to return a cached value if
set, only call Lich::Util.quiet_command_xml when the cache is unset, store
either the found name or nil into `@cached_account_name` and mark
`@cached_account_name_set`=true, and add an optional invalidation method (e.g.,
AccountInfo.invalidate_name_cache) so callers like record_transaction and
estimate_loot_cap will use the cached outcome and avoid repeated account
commands.
1 participant
…tion
Summary by CodeRabbit
New Features
Improvements
Chores