Skip to content

Commit f3d051d

Browse files
author
Tehan
committed
fix(skill-memory): address council should-level findings
- budgetFill now counts per-note XML framing (~20 tokens) so the rendered <skill-memory> block stays within max_tokens instead of ~13% overshoot (rev-1). - clamp effective pinned budget to min(max_pinned_tokens, max_tokens) so the default 4000>1500 can't imply pinned gets more room than the whole block (rev-2). - ctx_skill_recall: derive tier via dirname(resolvedPath) instead of a fragile .replace('/SKILL.md','') (rev-2). Updated the budget-truncation test for the framing-inclusive math.
1 parent 942cd32 commit f3d051d

3 files changed

Lines changed: 23 additions & 10 deletions

File tree

packages/plugin/src/features/magic-context/skill-memory/recall.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,13 @@ describe("flatRecall", () => {
5353
createdAt: now - i * 1000,
5454
});
5555
}
56-
// Token-budget truncation test (arithmetic verified):
57-
// delta = "note N — " (9 chars) + 40 "x"s = 49 chars → Math.ceil(49/4) = 13 tokens each.
58-
// maxTokens: 30 → first note fits (13 ≤ 30), second note fits (13+13=26 ≤ 30),
59-
// third note would exceed (26+13=39 > 30) → exactly 2 notes fit.
56+
// Token-budget truncation test (arithmetic verified, framing-inclusive):
57+
// delta = "note N — " (9 chars) + 40 "x"s = 49 chars → Math.ceil(49/4) = 13 tokens,
58+
// plus NOTE_FRAMING_TOKENS (20) for the <note>…</note> wrapper = 33 tokens each.
59+
// maxTokens: 70 → 1st fits (33 ≤ 70), 2nd fits (33+33=66 ≤ 70),
60+
// 3rd would exceed (66+33=99 > 70) → exactly 2 notes fit.
6061
const notes = flatRecall(db, "tdd", "global", "git:abc", {
61-
maxTokens: 30, // 2 notes × 13 tokens = 2630; 3rd note would push to 39 > 30
62+
maxTokens: 70, // 2 notes × 33 tokens = 6670; 3rd would push to 99 > 70
6263
maxPinnedTokens: 4000,
6364
});
6465
expect(notes.length).toBe(2);

packages/plugin/src/features/magic-context/skill-memory/recall.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ export interface FlatRecallOptions {
1919
maxPinnedTokens: number;
2020
}
2121

22+
// Per-note <note kind=… intent=… hit_count=… pinned=…><delta>…</delta></note>
23+
// framing that budgetFill's delta-only estimate would otherwise ignore (~80
24+
// chars ≈ 20 tokens/note). Counting it keeps the rendered block within max_tokens
25+
// instead of overshooting ~13% on a 10-note block (rev-1 S1).
26+
const NOTE_FRAMING_TOKENS = 20;
27+
2228
// Rough token estimate: 1 token ≈ 4 chars (conservative for XML overhead)
2329
function estimateTokens(text: string): number {
2430
return Math.ceil(text.length / 4);
@@ -115,11 +121,15 @@ function budgetFill(
115121
const result: SkillMemoryNote[] = [];
116122
let pinnedTokens = 0;
117123
let totalTokens = 0;
124+
// The total budget is the hard ceiling, so the pinned sub-budget can never
125+
// exceed it — clamp so the default max_pinned_tokens (4000) > max_tokens
126+
// (1500) can't imply pinned notes get more room than the whole block (rev-2).
127+
const effectiveMaxPinned = Math.min(maxPinnedTokens, maxTokens);
118128

119129
for (const note of notes) {
120-
const tokens = estimateTokens(note.delta);
130+
const tokens = estimateTokens(note.delta) + NOTE_FRAMING_TOKENS;
121131
if (note.pinned === 1) {
122-
if (pinnedTokens + tokens > maxPinnedTokens) continue;
132+
if (pinnedTokens + tokens > effectiveMaxPinned) continue;
123133
pinnedTokens += tokens;
124134
}
125135
if (totalTokens + tokens > maxTokens) continue;

packages/plugin/src/tools/ctx-skill-recall/tools.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { readFileSync } from "node:fs";
2+
import { dirname } from "node:path";
23
import { type ToolContext, type ToolDefinition, tool } from "@opencode-ai/plugin";
34
import { resolveProjectIdentity } from "../../features/magic-context/memory/project-identity";
45
import { parseFrontmatterConfig } from "../../features/magic-context/skill-memory/frontmatter";
@@ -151,16 +152,17 @@ export function createCtxSkillRecallTool(deps: CtxSkillRecallToolDeps): ToolDefi
151152
`SKILL.md not found for '${args.skill}' in any known skill directory. ` +
152153
`Load the skill first with the skill tool, or verify the skill name is correct. ` +
153154
`Searched: project .opencode/skill/, .opencode/skills/, .agents/skills/, .claude/skills/; ` +
154-
`global ~/.config/opencode/skills/, ~/.agents/skills/, ~/.claude/skills/.`
155+
`global ~/.config/opencode/skill/, ~/.config/opencode/skills/, ~/.agents/skills/, ~/.claude/skills/.`
155156
);
156157
}
157158

158159
// Parse frontmatter from on-disk SKILL.md
159160
frontmatterConfig = rawSkillContent
160161
? parseFrontmatterConfig(rawSkillContent)
161162
: null;
162-
// Derive tier from resolved path
163-
tier = deriveSkillTier(resolvedPath.replace("/SKILL.md", ""));
163+
// Derive tier from the skill's directory (dirname, not a fragile
164+
// string replace that would mis-handle a path containing "SKILL.md").
165+
tier = deriveSkillTier(dirname(resolvedPath));
164166
}
165167

166168
if (!frontmatterConfig?.enabled) {

0 commit comments

Comments
 (0)