Skip to content

fix(retrieve): prioritize retained call source in compact snippets - #750

Merged
mohanagy merged 2 commits into
nextfrom
fix/740-compact-call-snippet-allocation
Sep 7, 2026
Merged

mohanagy merged 2 commits into
nextfrom
fix/740-compact-call-snippet-allocation

Conversation

@mohanagy

@mohanagy mohanagy commented Sep 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

Compact retrieval could retain both ends of a call while omitting a helper's existing source because that helper fell beyond the first eight snippet slots. With the snippet limit omitted, compact retrieval now prioritizes source-bearing endpoints of retained calls relationships, then fills the remaining slots in node order. Node order, the eight-slot default, and the 3,000-token snippet budget are unchanged.

Explicit snippet limits remain positional. Missing, blank, duplicate, or source-less endpoint IDs cannot create a preference. Verbose output and promoted execution slices retain their existing allocation. The tool parameter description documents default versus explicit behavior.

Candidate: 1ce26963ebf273c8d337fb4cc2a0c8abe8cb3bfa on base 214f70bf4c4edf820a0c7651ccd81dba3ca7ac1e.

Testing

  • Focused guarded Vitest run: 262 tests across 10 files, including 27 allocation tests and existing retrieval, source-preservation, context-pack, slice, and stdio coverage.
  • Typecheck: tsc --noEmit.
  • Build: tsc -p tsconfig.build.json.
  • Three mutation controls caught positional-default restoration, preference leaking into explicit limits, and invalid-endpoint acceptance.
  • Frozen 72-case base/candidate comparison (144 calls): source coverage among selected helpers 102/108 → 108/108; complete compact cases 32/72 → 36/72; zero frozen required-fact losses or node membership/order changes. All six source-preservation consumers pass. Required-ID discovery remains 54/72; the full inventory has 144 helper obligations.
  • Description correction: 120 affected tests across five files, typecheck and build pass on the corrected source. Core tool-list size is now 3,400 bytes (limit unchanged); strict 1,468; full 12,889.
  • Corrected-head CI run 34106342385: all six Linux/macOS/Windows × Node 20/22 jobs pass at 1ce26963. All six logs verify the expected synthetic merge checkout and candidate tree. The previous macOS / Node 20 Gemini-hook EPIPE did not recur; its precise cause remains unproven. Original failed run 34097545677 and all failure signatures remain retained; no unchanged-head rerun.
  • CodeRabbit reviewed the exact corrected head with no actionable code findings. Its docstring-coverage warning is recorded as a non-blocking documentation metric outside this change's acceptance criteria.
  • Fresh independent Codex CLI FINAL: GO-740-ALLOCATION on 1ce26963 / tree 1078d3cb. The reviewer accepted A1–A8 and the explicit timing/carry-forward classification. This was a packet review; the coordinator executed the tests and hash checks.
  • Evidence from the 72-case matrix and three mutation controls is carried forward from 2f76a8e7, not newly executed. Only the description string changed afterward; the matrix's 67 reachable source files and all 271 external inputs are identical. All exported tool/profile values match except that description, and clean allocation/stdio/schema tests were rerun.
  • Packaging check: not applicable; packaging and install behavior are unchanged.

The original matrix's additional literal raw-payload equality assertion exited 1. The only raw differences are four measured recovery.attempts[0].elapsed_ms values: 5→4, 5→7, 8→6, and 9→8 ms. All other raw values and all direct results match; the timing-owner source is identical at both commits. Original payloads and the failed assertion are retained, and this explicit timing classification is submitted to final review. No performance claim or threshold exception is made.

These are exposed synthetic development cases with manually declared graphs. They do not establish extraction quality, agent-answer gains, or resolution of the remaining owner-input and neutral-discovery cases. Issue #740 remains open.

Checklist

  • Updated the public tool parameter description for the user-visible behavior.
  • Added regression tests and mutation controls.
  • No private corpora, secrets, credentials, proprietary prompts, sensitive logs, or generated evidence artifacts are included in this PR.
  • Change is limited to retrieval allocation, the parameter description, and its tests.

Related issues

Refs #740. Follows #747. Allocation contract: #740 (comment)

@coderabbitai

coderabbitai Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: ca703516-bb5e-4371-b91b-76603dbd2065

📥 Commits

Reviewing files that changed from the base of the PR and between 214f70b and 1ce2696.

📒 Files selected for processing (3)
  • src/runtime/retrieve.ts
  • src/runtime/stdio/definitions.ts
  • tests/unit/retrieve-snippet-allocation.test.ts

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

Compact retrieval now prioritizes call-end nodes with available snippets when no explicit snippet limit is provided. Explicit limits and promoted slices retain positional allocation. Tests cover allocation, validation, budgets, and stdio output integrity.

Changes

Snippet allocation

Layer / File(s) Summary
Snippet allocation engine
src/runtime/retrieve.ts, tests/unit/retrieve-snippet-allocation.test.ts
Snippet budgeting accepts eligible node indexes. Call endpoints with available snippets receive priority. Tests cover endpoint identity, duplicates, invalid relationships, and missing snippets.
Compact retrieval integration
src/runtime/retrieve.ts, src/runtime/stdio/definitions.ts, tests/unit/retrieve-snippet-allocation.test.ts
Compact retrieval uses call-endpoint prioritization only without an explicit limit and for non-promoted slices. The stdio schema describes the default behavior.
Controls, budgets, and output validation
tests/unit/retrieve-snippet-allocation.test.ts
Tests cover option aliases, validation, budget limits, truncation, promoted slices, and removal of dangling stdio references.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 1ce26

Compact retrieval changes default snippet selection while preserving explicit limits and existing budgets; no actionable product risk is currently established.

Sequence Diagram(s)

sequenceDiagram
  participant RetrieveCaller
  participant CompactRetrieval
  participant SnippetAllocator
  participant StdioOutput
  RetrieveCaller->>CompactRetrieval: request compact retrieval
  CompactRetrieval->>SnippetAllocator: select call-endpoint snippets
  SnippetAllocator->>CompactRetrieval: return budgeted nodes
  CompactRetrieval->>StdioOutput: emit reduced result
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly identifies the main change: compact retrieval now prioritizes retained call sources in snippets.
Description check ✅ Passed The description includes all required sections, explains the behavior and scope, documents testing and validation, completes the checklist, and references the related issue.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/740-compact-call-snippet-allocation

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

@mohanagy
mohanagy marked this pull request as ready for review September 7, 2026 09:30
@mohanagy
mohanagy merged commit 9214c07 into next Sep 7, 2026
7 checks passed
@mohanagy
mohanagy deleted the fix/740-compact-call-snippet-allocation branch September 7, 2026 12:14
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