Skip to content

fix(api): refresh the tile item cache on the client thread - #1881

Merged
chsami merged 1 commit into
developmentfrom
claude/tile-item-cache-client-thread
Oct 4, 2026
Merged

chsami merged 1 commit into
developmentfrom
claude/tile-item-cache-client-thread

Conversation

@chsami

@chsami chsami commented Oct 4, 2026

Copy link
Copy Markdown
Owner

CodeRabbit flagged this on #1880. Rs2TileItemQueryable reads its initial source in its constructor (AbstractEntityQueryable calls initialSource(), which calls Rs2TileItemCache.getStream()). Any Microbot.getRs2TileItemCache().query() built off the client thread therefore refreshed the cache from Scene#getTiles() / Tile#getGroundItems() on the script thread, even when it ended in nearestOnClientThread(). Rs2GroundItem.interact and several existing callers do exactly this.

Change

  • getStream() now runs the tick check and the whole scene scan inside clientThread.runOnClientThreadOptional(...). On the client thread this runs inline. Off it, the call waits for the client thread.
  • tileItems and lastUpdateTick are volatile. The published list is a fresh snapshot that is never mutated afterwards, so other threads can safely stream it.
  • If the client-thread task fails or times out, getStream() returns an empty stream. It returned empty before when there was no local player.
  • The seven Rs2TileItemCache#getStream() entries are removed from the client-thread guardrail baseline, because they are no longer violations. The regenerated baseline also re-sorts one Rs2GrandExchange line. Nothing else changed.

Trade-off: a query built off the client thread now waits for one client-thread task, even when the cache is already fresh for the current tick. The freshness check needs client.getTickCount(), which the guardrail requires to run on the client thread.

Rs2TileObjectCache.getStream() has the same off-thread refresh. I've left it out of this PR to keep it scoped; it can follow the same pattern.

Validation

  • New Rs2TileItemCacheTest covers three cases:
    • Every client and scene read happens inside the client-thread task.
    • A second call in the same tick reuses the snapshot, and the next tick rescans.
    • A failed client-thread task returns an empty stream without reading the client.
  • ClientThreadGuardrailTest and QueryableTerminalGuardrailTest pass.
  • ./ci/build.sh passes: buildAll plus 1897 unit tests, 0 failures, 1 skipped.
  • No live game testing.

🤖 Generated with Claude Code

Rs2TileItemQueryable reads its initial source in the constructor, so
query() refreshed Rs2TileItemCache from scene tiles on whatever thread
built the query. Run the tick check and scene scan inside
runOnClientThreadOptional, publish the snapshot through volatile fields,
and return an empty stream if the client-thread task fails. Drop the
seven resolved getStream() entries from the guardrail baseline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
docs/development.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c772e333-1ebb-4914-a92f-a767ef3c75a3
📥 Commits

Reviewing files that changed from the base of the PR and between 275f936 and c6dd872.

📒 Files selected for processing (3)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/api/tileitem/Rs2TileItemCache.java
  • runelite-client/src/test/java/net/runelite/client/plugins/microbot/api/tileitem/Rs2TileItemCacheTest.java
  • runelite-client/src/test/resources/threadsafety/client-thread-guardrail-baseline.txt

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


Walkthrough

Tile item cache refresh and retrieval now run through ClientThread.runOnClientThreadOptional. The cache uses volatile fields and returns an empty stream when the operation has no result. New tests cover client-thread scene access, same-tick cache reuse, rescanning on a new tick, and empty results. The guardrail baseline removes seven tile cache violations and adds a Grand Exchange Widget#getBounds() violation.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to c6dd8

The tile-item cache change is mergeable after normal checks; no actionable regression remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c6dd8

The change strengthens thread ownership without adding permissions or interaction actions. Remaining risk is limited to callers waiting for the client thread and receiving empty results when execution fails.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed operation reads registered world views within the running client. Existing ground-item request parameters filter these results, and the existing pickup consumer invokes downstream interaction authority. The cache change adds neither a request route nor an interaction sink.

Trust Boundaries and Controls

  • observed — The boundary strengthened here is client-state thread ownership, not authentication or authorization. Tick, player, world-view, scene, and ground-item reads now execute together on the client thread; off-thread callers receive the resulting list through the scheduling helper.

Resilience and Maintainability Implications

  • inferred — Each scan builds a private list and publishes it only after completion, followed by its captured tick. Serialized client-thread execution prevents overlapping refreshes. A scan exception before publication leaves the previous cache intact for retry. Interruption can leave work queued, and a timed-out running task may finish later, but any such publication still follows a complete scan rather than exposing partial results.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: refreshing the tile item cache on the client thread.
Description check ✅ Passed The description explains the client-thread issue, the cache changes, and the validation performed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · 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.

@chsami
chsami merged commit b9e1b2f into development Oct 4, 2026
3 checks passed
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.

1 participant