feat: automatic skill detection and extraction prompt injection#2
feat: automatic skill detection and extraction prompt injection#2bhodgens wants to merge 2 commits intodeiviuds:mainfrom
Conversation
Summary of ChangesHello @bhodgens, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Summary by CodeRabbit
WalkthroughThe changes implement a skill detection and staging system that identifies potential skills from session observations, stages them persistently, and injects them into system prompts. Simultaneously, the public API surface is simplified by removing most re-exports from the main index file. Changes
Sequence DiagramsequenceDiagram
participant Session as Session Handler
participant Detector as Skill Detector
participant Staging as Staging Manager
participant Plugin as Plugin/Prompt
participant Next as Next Session
Session->>Detector: detectSkillCandidates(observations, sessionId)
Detector->>Detector: Classify observations into patterns<br/>(problems, solutions, errors, fixes, etc.)
Detector->>Detector: Generate candidates via 4 patterns<br/>(Problem→Solution, Error→Fix, etc.)
Detector->>Detector: Deduplicate & sort by confidence
Detector-->>Session: Return SkillCandidate[]
Session->>Staging: savePendingCandidates(directory, candidates)
Staging->>Staging: Load existing staged data
Staging->>Staging: Merge new with un-injected candidates
Staging->>Staging: Deduplicate by title, cap at 10
Staging-->>Session: Persist staging file
Note over Plugin: On next session start
Plugin->>Staging: loadPendingCandidates(directory)
Staging-->>Plugin: Return StagedCandidates
Plugin->>Plugin: Format candidates for system prompt
Plugin->>Staging: markInjected(directory)
Next->>Next: System prompt includes skill context
Note over Staging: On session idle
Session->>Staging: clearPendingCandidates(directory)
Staging-->>Session: Remove staging file
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/plugin.ts (1)
96-98:⚠️ Potential issue | 🔴 CriticalBug:
skillCandidatesInjectedis not reset onsession.created, so injection only works for the first session in the plugin's lifecycle.
sessionSummaryGeneratedis correctly reset tofalseonsession.created(line 97), butskillCandidatesInjectedis not. If the plugin process survives across sessions, candidates staged by Session N'shandleSessionEndwill never be injected into Session N+1's system prompt because the flag remainstruefrom Session N.🐛 Proposed fix
if (event.type === "session.created") { sessionSummaryGenerated = false + skillCandidatesInjected = false const memoryPath = resolve(directory, ".claude/mind.mv2")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugin.ts` around lines 96 - 98, The flag skillCandidatesInjected must be reset on new sessions like sessionSummaryGenerated is; in the event.type === "session.created" branch set skillCandidatesInjected = false so candidates staged by handleSessionEnd can be injected into the next session's system prompt—locate the session.created handling block in plugin.ts (where sessionSummaryGenerated is reset) and add the reset of skillCandidatesInjected there.
🧹 Nitpick comments (4)
src/utils/skill-staging.ts (1)
59-61: Unnecessary dynamic import ofunlink— it can be statically imported at line 1.
readFile,writeFile, andmkdirare already statically imported from"node:fs/promises"at line 1. The dynamicimport("node:fs/promises")forunlinkis inconsistent and adds needless overhead.♻️ Proposed fix
-import { readFile, writeFile, mkdir } from "node:fs/promises" +import { readFile, writeFile, mkdir, unlink } from "node:fs/promises"Then in
clearPendingCandidates:try { - const { unlink } = await import("node:fs/promises") await unlink(path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/skill-staging.ts` around lines 59 - 61, In clearPendingCandidates, remove the dynamic import("node:fs/promises") and instead add unlink to the existing static imports (alongside readFile, writeFile, mkdir) at the top; then call unlink(path) directly in the try block — this eliminates the unnecessary runtime import and keeps imports consistent for unlink, readFile, writeFile, and mkdir.src/hooks/skill-detector.ts (2)
135-154: Timestamp guard uses<=, which excludes distinct problem→solution pairs that share the same timestamp.Line 137 skips when
solution.timestamp <= problem.timestamp. If two distinct observations occur in the same millisecond (or both lack timestamps and default to 0), a valid pair is missed. This is a conservative tradeoff; just noting it in case rapid-fire observations are common.♻️ Alternative: use strict `<` and add identity check
- if ((solution.timestamp ?? 0) <= (problem.timestamp ?? 0)) continue + if (solution === problem) continue + if ((solution.timestamp ?? 0) < (problem.timestamp ?? 0)) continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/skill-detector.ts` around lines 135 - 154, The timestamp guard in the nested loops over win.problems and win.solutions (checking (solution.timestamp ?? 0) <= (problem.timestamp ?? 0)) incorrectly excludes distinct problem→solution pairs that share the exact timestamp; change the comparison to a strict `<` and add an identity check so pairs with equal timestamps are allowed when solution and problem are not the same observation (e.g., compare unique identifiers or object identity of solution vs problem before continuing). Update the condition in the loop where (solution.timestamp ?? 0) and (problem.timestamp ?? 0) are compared to use `<` plus a check like solution !== problem or solution.id !== problem.id to preserve ordering while not dropping same-timestamp distinct matches.
50-66: Observation double-classification: an observation can be both a problem and a solution.An observation of type
"solution"or"bugfix"whose content contains an ERROR_SIGNAL word (e.g., "Fixed the error in auth") will be pushed intowin.problems,win.errors,win.solutions, and potentiallywin.fixes. This is handled downstream — Pattern 1's timestamp guard (<=) prevents self-pairing sincesolution.timestamp === problem.timestampfor the same object — but it's subtle and worth a brief inline comment for future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/skill-detector.ts` around lines 50 - 66, The current classification loop can push the same observation into both problem/error and solution/fix buckets (e.g., a "solution" whose text contains an ERROR_SIGNAL); add a concise inline comment near the logic in the loop (around the checks that use observations, ERROR_SIGNALS, FIX_SIGNALS and the pushes to win.problems, win.errors, win.solutions, win.fixes) explaining that double-classification is intentional and safe because downstream pairing logic (Pattern 1) uses the timestamp guard (<=) to avoid self-pairing for the same observation; this will help future maintainers understand why we allow overlapping pushes rather than trying to de-duplicate here.src/plugin.ts (1)
156-166: SettingskillCandidatesInjected = truebefore the load attempt prevents retry on transient failure.If
loadPendingCandidatesthrows or returns null due to a transient I/O error, the flag is alreadytrueand no further attempts will be made for the rest of the session. Consider moving the flag assignment to after the successful injection.♻️ Suggested reorder
if (!skillCandidatesInjected) { - skillCandidatesInjected = true const staged = await loadPendingCandidates(directory) if (staged?.candidates?.length && !staged.injectedAt) { output.system.push(formatSkillCandidatesForSystem(staged.candidates)) await markInjected(directory) + skillCandidatesInjected = true debug(`Injected ${staged.candidates.length} skill candidate(s) into system prompt`) + } else { + skillCandidatesInjected = true // no candidates to inject, stop checking } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugin.ts` around lines 156 - 166, The code sets skillCandidatesInjected = true before attempting loadPendingCandidates, which prevents retries on transient failures; modify the flow in the block that references skillCandidatesInjected, loadPendingCandidates, formatSkillCandidatesForSystem, markInjected, output.system, and debug so the flag is only set after a successful injection: call loadPendingCandidates(directory) first, check staged is non-null and staged.candidates.length > 0 and !staged.injectedAt, push the formatted candidates to output.system, await markInjected(directory), call debug(...), and only then set skillCandidatesInjected = true; ensure the flag is not set if loadPendingCandidates throws or returns invalid data so retries can occur later in the session.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/plugin.ts`:
- Around line 96-98: The flag skillCandidatesInjected must be reset on new
sessions like sessionSummaryGenerated is; in the event.type ===
"session.created" branch set skillCandidatesInjected = false so candidates
staged by handleSessionEnd can be injected into the next session's system
prompt—locate the session.created handling block in plugin.ts (where
sessionSummaryGenerated is reset) and add the reset of skillCandidatesInjected
there.
---
Nitpick comments:
In `@src/hooks/skill-detector.ts`:
- Around line 135-154: The timestamp guard in the nested loops over win.problems
and win.solutions (checking (solution.timestamp ?? 0) <= (problem.timestamp ??
0)) incorrectly excludes distinct problem→solution pairs that share the exact
timestamp; change the comparison to a strict `<` and add an identity check so
pairs with equal timestamps are allowed when solution and problem are not the
same observation (e.g., compare unique identifiers or object identity of
solution vs problem before continuing). Update the condition in the loop where
(solution.timestamp ?? 0) and (problem.timestamp ?? 0) are compared to use `<`
plus a check like solution !== problem or solution.id !== problem.id to preserve
ordering while not dropping same-timestamp distinct matches.
- Around line 50-66: The current classification loop can push the same
observation into both problem/error and solution/fix buckets (e.g., a "solution"
whose text contains an ERROR_SIGNAL); add a concise inline comment near the
logic in the loop (around the checks that use observations, ERROR_SIGNALS,
FIX_SIGNALS and the pushes to win.problems, win.errors, win.solutions,
win.fixes) explaining that double-classification is intentional and safe because
downstream pairing logic (Pattern 1) uses the timestamp guard (<=) to avoid
self-pairing for the same observation; this will help future maintainers
understand why we allow overlapping pushes rather than trying to de-duplicate
here.
In `@src/plugin.ts`:
- Around line 156-166: The code sets skillCandidatesInjected = true before
attempting loadPendingCandidates, which prevents retries on transient failures;
modify the flow in the block that references skillCandidatesInjected,
loadPendingCandidates, formatSkillCandidatesForSystem, markInjected,
output.system, and debug so the flag is only set after a successful injection:
call loadPendingCandidates(directory) first, check staged is non-null and
staged.candidates.length > 0 and !staged.injectedAt, push the formatted
candidates to output.system, await markInjected(directory), call debug(...), and
only then set skillCandidatesInjected = true; ensure the flag is not set if
loadPendingCandidates throws or returns invalid data so retries can occur later
in the session.
In `@src/utils/skill-staging.ts`:
- Around line 59-61: In clearPendingCandidates, remove the dynamic
import("node:fs/promises") and instead add unlink to the existing static imports
(alongside readFile, writeFile, mkdir) at the top; then call unlink(path)
directly in the try block — this eliminates the unnecessary runtime import and
keeps imports consistent for unlink, readFile, writeFile, and mkdir.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an automatic skill detection and extraction feature, which is a significant enhancement to the opencode-brain plugin. The implementation is well-structured, separating concerns into skill-detector.ts for heuristic pattern matching, skill-staging.ts for cross-session persistence, and integrating these into session-end.ts and plugin.ts. The logic for detecting skill candidates, handling cross-session staging, and injecting them into the system prompt for agent evaluation is robust and thoughtfully designed. The changes to src/index.ts to export only the default plugin align with the plugin loader compatibility requirements. Overall, this is a valuable addition that enables a continuous learning loop for the agent.
| if (!existsSync(path)) return | ||
|
|
||
| try { | ||
| const { unlink } = await import("node:fs/promises") |
There was a problem hiding this comment.
The dynamic import for unlink on this line is generally less performant and can make code analysis harder compared to a static import. For a utility that is always needed when this function is called, it's better to use a static import at the top of the file for clarity and consistency.
import { readFile, writeFile, mkdir, unlink } from "node:fs/promises"1 participant
Summary
Adds automatic skill detection to opencode-brain. At session end, the plugin analyzes observations for patterns that suggest extractable, reusable knowledge. Detected candidates are staged to disk and injected into the next session's system prompt, prompting the agent to evaluate and extract them as SKILL.md files.
This enables a continuous learning loop: work produces observations -> observations are analyzed for patterns -> candidates are surfaced to the agent -> agent creates reusable skills.
How it works
Session N (detection):
session.idle).claude/mind-skills-pending.jsonSession N+1 (injection):
experimental.chat.system.transformcall, plugin checks for pending candidates<openception-skill-extraction>block into the system promptNew files
src/hooks/skill-detector.ts— 4 pattern detectors with confidence scoring (high/medium), dedup, capped at 5 candidates per sessionsrc/utils/skill-staging.ts— Read/write/merge/clear pending candidates on disk, cross-session stacking with dedup, cap at 10 totalModified files
src/hooks/session-end.ts— Calls skill detector after session summary generationsrc/plugin.ts— Injects pending candidates into system prompt (once per session), clears staging at session endDesign decisions
skillCandidatesInjectedflag ensures candidates are injected exactly once per plugin lifecycle, not on every message.Depends on