Skip to content

docs: fix doc drift in pending_signature_queue_len#3271

Merged
netrome merged 1 commit into
mainfrom
3270-fix-doc-drift-in-pending_signature_queue_len
May 18, 2026
Merged

docs: fix doc drift in pending_signature_queue_len#3271
netrome merged 1 commit into
mainfrom
3270-fix-doc-drift-in-pending_signature_queue_len

Conversation

@netrome

@netrome netrome commented May 18, 2026

Copy link
Copy Markdown
Collaborator

closes #3270

The dual-map drain refactor (#3232) removed the legacy→new migration
that the original comment described, so the "post-upgrade pushes drain
it into the head of the new queue" reasoning no longer holds. The view
itself is still correct for sandbox use — only the *why* needed a tweak.

Co-Authored-By: Barak Einav <barakeinav@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 18, 2026 11:20
@netrome netrome linked an issue May 18, 2026 that may be closed by this pull request

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes documentation drift for the sandbox-only pending_signature_queue_len view method, clarifying that it only consults the fan-out map and is authoritative for fresh sandbox state.

Changes:

  • Updates the method comment to remove the outdated claim that post-upgrade pushes drain legacy entries into the new queue.
  • Clarifies the legacy map is intentionally not consulted by this sandbox helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@claude

claude Bot commented May 18, 2026

Copy link
Copy Markdown

Pull request overview

This PR fixes doc drift in the pending_signature_queue_len sandbox helper. The previous comment claimed that "post-upgrade pushes drain [the legacy single-yield map] into the head of the new queue," which is not what the code actually does — push_pending_yield always writes to the new fan-out map and leaves the legacy map untouched (crates/contract/src/pending_requests.rs:96). The corrected wording scopes the "authoritative length" claim to sandbox tests that start from fresh state, where no legacy entries can exist.

Changes:

  • Reword the doc comment on pending_signature_queue_len to remove the incorrect "drain into the head of the new queue" claim and replace it with the accurate fresh-state qualifier.

Reviewed changes

Per-file summary
File Description
crates/contract/src/sandbox_test_methods.rs Doc-only change on pending_signature_queue_len; behavior unchanged.

Findings

No blocking or non-blocking issues. The new wording is consistent with push_pending_yield / resolve_yields_for / pop_one_yield_for in crates/contract/src/pending_requests.rs, and pending_ckd_queue_len directly below already refers back to this comment via "same rationale," so the single-location fix is sufficient.

✅ Approved

@netrome netrome added this pull request to the merge queue May 18, 2026
Merged via the queue into main with commit 393f14f May 18, 2026
18 checks passed
@netrome netrome deleted the 3270-fix-doc-drift-in-pending_signature_queue_len branch May 18, 2026 14:36
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.

Fix doc drift in pending_signature_queue_len

5 participants