feat(memory): proportional token budget allocation#165
Conversation
…tion starvation Each section now receives a fixed proportion of maxTokens (agent 30%, project 25%, feedback 20%, user 15%, reference 10%) instead of the remaining budget, so a large agent section can no longer starve downstream sections. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
New PR opened -- automated review will run on the next push. To trigger a manual review, comment |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Pull request overview
Implements proportional token budget allocation across memory sections to prevent early sections (especially agent-specific) from consuming the entire memory token budget, and expands test coverage around section fairness.
Changes:
- Add a default proportional budget allocation (Agent/Project/Feedback/User/Reference) and apply fixed per-section budgets during injection.
- Track
allIncludedIdsacross all injected sections for accurateincrementAccessBatchaccess count updates. - Add tests validating that multiple sections can be included, that large agent memory doesn’t starve general memory, and that empty sections are omitted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/opencode/src/memory/injector.ts | Switches from rolling budget to proportional per-section budgets and updates access-count tracking IDs. |
| packages/opencode/test/memory/injector.test.ts | Adds tests for multi-section inclusion, starvation prevention, and empty-section behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -84,21 +99,20 @@ export namespace MemoryInjector { | |||
| ["Feedback & Patterns", feedbackEntries], | |||
| ["Reference", referenceEntries], | |||
| ] as const) { | |||
| if (group.length === 0 || tokenBudget <= 0) continue | |||
| const section = buildSection(title, group, tokenBudget) | |||
| if (group.length === 0) continue | |||
| const section = buildSection(title, group, generalBudgets[title]) | |||
| if (section.text) { | |||
| sections.push(section.text) | |||
| tokenBudget -= section.tokens | |||
| injectedIds.push(...section.includedIds) | |||
| allIncludedIds.push(...section.includedIds) | |||
| } | |||
There was a problem hiding this comment.
With fixed per-section budgets, any unused budget in an empty (or short) section is currently lost rather than being reallocated to sections that have remaining entries. This can significantly reduce injected context (e.g., if only Project memories exist, they’ll still be limited to ~25% of maxTokens), and it also contradicts the PR summary/test-plan claim that empty sections shouldn’t waste budget. Consider re-normalizing the allocation over the set of non-empty sections (and over agent/general depending on whether agent entries exist), or allow unused section budget to roll over into later sections while keeping per-section caps to prevent starvation.
| @@ -84,21 +99,20 @@ export namespace MemoryInjector { | |||
| ["Feedback & Patterns", feedbackEntries], | |||
| ["Reference", referenceEntries], | |||
| ] as const) { | |||
| if (group.length === 0 || tokenBudget <= 0) continue | |||
| const section = buildSection(title, group, tokenBudget) | |||
| if (group.length === 0) continue | |||
| const section = buildSection(title, group, generalBudgets[title]) | |||
| if (section.text) { | |||
| sections.push(section.text) | |||
There was a problem hiding this comment.
Per-section budgets are computed with Math.floor(maxTokens * allocation.*). For small maxTokens values this can produce token budgets that are smaller than the section header token cost (or even 0), causing buildSection to return an empty string even when there is relevant content and total maxTokens would allow it. Adding a minimum per non-empty section budget (e.g., enough for header + at least one entry) and adjusting the remaining budget distribution would avoid surprising “no memory injected” behavior under tight token configs.
| test("empty sections do not waste budget", async () => { | ||
| await using tmp = await tmpdir({ git: true }) | ||
| await Instance.provide({ | ||
| directory: tmp.path, | ||
| fn: async () => { | ||
| const ts = Date.now() | ||
| // Only seed project and feedback — user and reference are empty | ||
| Database.use((d) => { | ||
| seed(d, { id: `only-proj-${ts}`, projectPath: tmp.path, topic: "only-project", type: "project", content: "Only project entry" }) | ||
| seed(d, { id: `only-fb-${ts}`, projectPath: tmp.path, topic: "only-feedback", type: "feedback", content: "Only feedback entry" }) | ||
| }) | ||
|
|
||
| const result = await MemoryInjector.load() | ||
| expect(result).toBeDefined() | ||
| // Present sections appear | ||
| expect(result).toContain("only-project") | ||
| expect(result).toContain("only-feedback") | ||
| // Empty sections produce no header | ||
| expect(result).not.toContain("## User Preferences") | ||
| expect(result).not.toContain("## Reference") | ||
| }, |
There was a problem hiding this comment.
The test name "empty sections do not waste budget" is misleading relative to what is asserted (it only checks that headers for empty sections are omitted). If the intended behavior is budget reallocation, extend the test to verify that remaining sections can use the freed budget; otherwise rename the test to match the actual expectation (e.g., that empty sections are not rendered).
- Only reserve agent budget (30%) when agent entries actually exist - Redistribute unused budget proportionally among active sections - Fixes: no-agent invocations now use 100% of budget for general sections Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Sections that can't fit header+entries have their budget redistributed to viable sections in a second pass - Rename test to accurately describe assertion (empty sections → no headers) - Add test verifying budget redistribution under tight token config Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
DEFAULT_BUDGET_ALLOCATIONwith proportional percentages (Agent 30%, Project 25%, Feedback 20%, User 15%, Reference 10%)allIncludedIdsfrom each section for accurateincrementAccessBatchtrackingTest plan
Refs: #161
🤖 Generated with Claude Code