Add cortex insights tool#12
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant Tool as Tool Executor
participant Client as HTTP Client
participant API as Insights API
participant Formatter as Response Formatter
Tool->>Tool: Extract params (status, limit)
Tool->>Client: listInsights(status, limit)
Client->>API: GET /api/v1/insights?owner_id=...&status=...&limit=...
API-->>Client: { insights?: [...], count?: ... }
alt insights found and valid
Client-->>Formatter: insights array
Formatter->>Formatter: Format with tags
Formatter-->>Tool: formatted string list
else empty result
Formatter-->>Tool: "No insights found."
else null/undefined result
Formatter-->>Tool: "Failed to fetch insights."
else error thrown
Tool-->>Tool: catch error
Tool-->>Tool: "List insights failed: ..."
end
Tool-->>Tool: Return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| async listInsights(status = "pending", limit = 5) { | ||
| return this.get<{ insights?: Record<string, unknown>[]; count?: number }>( | ||
| `/api/v1/insights?owner_id=${encodeURIComponent(this.ownerId)}&status=${encodeURIComponent(status)}&limit=${encodeURIComponent(String(limit))}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
📝 Info: listInsights uses manual URL encoding instead of URLSearchParams
The new listInsights method at src/index.ts:363-367 manually constructs the query string using encodeURIComponent, whereas all other list methods (listCommitments at src/index.ts:355-358, listOpenLoops at src/index.ts:385-388, listContradictions at src/index.ts:325-329) use URLSearchParams. Both approaches produce correct output, but the inconsistency makes the codebase slightly harder to maintain. Not a bug, but a style divergence worth noting.
Was this helpful? React with 👍 or 👎 to provide feedback.
| api.registerTool( | ||
| { | ||
| name: "cortex_insights", | ||
| label: "Cortex Insights", | ||
| description: "List cross-system insights from behavioral + memory analysis. Shows patterns, correlations, and recommendations discovered by the dreaming engine.", | ||
| parameters: Type.Object({ | ||
| status: Type.Optional(Type.String({ description: "Filter by status: pending, accepted, or all" })), | ||
| limit: Type.Optional(Type.Number({ description: "Max results to return (default: 5)" })), | ||
| }), | ||
| async execute(_toolCallId: string, params: unknown): Promise<any> { | ||
| const { status, limit } = params as { status?: string; limit?: number }; | ||
| try { | ||
| const result = await client.listInsights(status ?? "pending", limit ?? 5); | ||
| if (!result) { | ||
| return { content: [{ type: "text" as const, text: "Failed to fetch insights." }] }; | ||
| } | ||
| const items = (result as any)?.insights ?? []; | ||
| if (!items.length) { | ||
| return { content: [{ type: "text" as const, text: "No insights found." }] }; | ||
| } | ||
| const text = items | ||
| .map((insight: any, i: number) => { | ||
| const conf = typeof insight.confidence === "number" ? ` (${Math.round(insight.confidence * 100)}%)` : ""; | ||
| const type = insight.insight_type ? ` [${insight.insight_type}]` : ""; | ||
| const statusTag = insight.status ? ` [${insight.status}]` : ""; | ||
| return `${i + 1}. ${insight.insight ?? insight.content ?? JSON.stringify(insight)}${conf}${type}${statusTag}`; | ||
| }) | ||
| .join("\n"); | ||
| return { content: [{ type: "text" as const, text: `Found ${items.length} insight(s):\n\n${text}` }] }; | ||
| } catch (err) { | ||
| return { content: [{ type: "text" as const, text: `List insights failed: ${String(err)}` }] }; | ||
| } | ||
| }, | ||
| }, | ||
| { name: "cortex_insights" }, | ||
| ); |
There was a problem hiding this comment.
📝 Info: cortex_insights tool does not expose owner_id parameter unlike peer tools
Most similar tools (e.g., cortex_list_commitments, cortex_list_open_loops, cortex_list_contradictions) expose an optional owner_id parameter allowing callers to override the default owner namespace. The cortex_insights tool and its underlying listInsights method always use this.ownerId with no override. This may be intentional (insights are always scoped to the configured owner) but it's a deviation from the pattern of the sibling tools. If multi-owner insight queries are ever needed, this would require a change.
Was this helpful? React with 👍 or 👎 to provide feedback.
| }, | ||
| "injectionFormat": { | ||
| "type": "string", | ||
| "enum": [ | ||
| "v1", | ||
| "v2" | ||
| ], | ||
| "default": "v1", | ||
| "description": "Memory injection format version" | ||
| }, | ||
| "showConflicts": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show conflict markers in v2 format" | ||
| }, | ||
| "showRelations": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show relation hints in v2 format" | ||
| }, | ||
| "dedup": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Deduplicate similar memories in v2 format" | ||
| } |
There was a problem hiding this comment.
📝 Info: Config schema in openclaw.plugin.json catches up to already-shipped code
The new config properties (injectionFormat, showConflicts, showRelations, dedup) added to openclaw.plugin.json are already present in the EvaMemoryConfig interface and parseConfig at src/index.ts:55-58 and src/index.ts:122-125, shipped in the prior PR #10 (feat: add v2 memory injection formatting). This PR is simply aligning the JSON schema declaration with the already-functional code, which is a good catch-up.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pull request overview
Adds a new Cortex “insights” capability to the OpenClaw Cortex plugin by extending the HTTP client, registering a new tool, and updating published artifacts/manifest to expose it.
Changes:
- Add
CortexClient.listInsights()for the/api/v1/insightsendpoint. - Register a new
cortex_insightstool and include it inopenclaw.plugin.json. - Rebuild
dist/outputs.
Reviewed changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Adds the listInsights client method and registers the cortex_insights tool. |
| openclaw.plugin.json | Exposes cortex_insights in the manifest and updates config schema formatting/fields. |
| dist/index.js | Compiled JS including the new insights client + tool registration. |
| dist/index.js.map | Updated sourcemap for rebuilt JS bundle. |
| dist/index.d.ts.map | Updated declaration map for rebuilt typings output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "injectionFormat": { | ||
| "type": "string", | ||
| "enum": [ | ||
| "v1", | ||
| "v2" | ||
| ], | ||
| "default": "v1", | ||
| "description": "Memory injection format version" | ||
| }, | ||
| "showConflicts": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show conflict markers in v2 format" | ||
| }, | ||
| "showRelations": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show relation hints in v2 format" | ||
| }, | ||
| "dedup": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Deduplicate similar memories in v2 format" |
There was a problem hiding this comment.
The PR description mentions exposing the new cortex_insights tool in the manifest, but this diff also adds new config schema properties (injectionFormat, showConflicts, showRelations, dedup). If those additions are intentional, please update the PR description to reflect the broader manifest/config change so reviewers/users know about the new config options.
| return this.get<{ insights?: Record<string, unknown>[]; count?: number }>( | ||
| `/api/v1/insights?owner_id=${encodeURIComponent(this.ownerId)}&status=${encodeURIComponent(status)}&limit=${encodeURIComponent(String(limit))}`, |
There was a problem hiding this comment.
listInsights builds the query string manually, unlike the other list* methods that use URLSearchParams. To keep encoding consistent and avoid subtle query-string bugs as parameters evolve, build the URL with new URLSearchParams({ owner_id: ..., status: ..., limit: ... }) and interpolate ?${params} (similar to listCommitments / listOpenLoops).
| return this.get<{ insights?: Record<string, unknown>[]; count?: number }>( | |
| `/api/v1/insights?owner_id=${encodeURIComponent(this.ownerId)}&status=${encodeURIComponent(status)}&limit=${encodeURIComponent(String(limit))}`, | |
| const params = new URLSearchParams({ | |
| owner_id: this.ownerId, | |
| status, | |
| limit: String(limit), | |
| }); | |
| return this.get<{ insights?: Record<string, unknown>[]; count?: number }>( | |
| `/api/v1/insights?${params}`, |
| async listInsights(status = "pending", limit = 5) { | ||
| return this.get<{ insights?: Record<string, unknown>[]; count?: number }>( | ||
| `/api/v1/insights?owner_id=${encodeURIComponent(this.ownerId)}&status=${encodeURIComponent(status)}&limit=${encodeURIComponent(String(limit))}`, |
There was a problem hiding this comment.
listInsights is the only list-style CortexClient method here that does not accept an optional ownerId parameter (the other list methods do). If insights are namespaced like the other resources, consider adding ownerId?: string and defaulting to this.ownerId for API consistency and future flexibility.
| async listInsights(status = "pending", limit = 5) { | |
| return this.get<{ insights?: Record<string, unknown>[]; count?: number }>( | |
| `/api/v1/insights?owner_id=${encodeURIComponent(this.ownerId)}&status=${encodeURIComponent(status)}&limit=${encodeURIComponent(String(limit))}`, | |
| async listInsights(ownerId?: string, status = "pending", limit = 5) { | |
| return this.get<{ insights?: Record<string, unknown>[]; count?: number }>( | |
| `/api/v1/insights?owner_id=${encodeURIComponent(ownerId ?? this.ownerId)}&status=${encodeURIComponent(status)}&limit=${encodeURIComponent(String(limit))}`, |
| status: Type.Optional(Type.String({ description: "Filter by status: pending, accepted, or all" })), | ||
| limit: Type.Optional(Type.Number({ description: "Max results to return (default: 5)" })), | ||
| }), | ||
| async execute(_toolCallId: string, params: unknown): Promise<any> { | ||
| const { status, limit } = params as { status?: string; limit?: number }; | ||
| try { | ||
| const result = await client.listInsights(status ?? "pending", limit ?? 5); |
There was a problem hiding this comment.
The tool schema allows any string status and any numeric limit (including negative / non-integer / very large values). Since these directly control the API query and output size, constrain them in the TypeBox schema (e.g., literals/union for status, and an integer with min/max for limit) and/or clamp/validate before calling client.listInsights.
| status: Type.Optional(Type.String({ description: "Filter by status: pending, accepted, or all" })), | |
| limit: Type.Optional(Type.Number({ description: "Max results to return (default: 5)" })), | |
| }), | |
| async execute(_toolCallId: string, params: unknown): Promise<any> { | |
| const { status, limit } = params as { status?: string; limit?: number }; | |
| try { | |
| const result = await client.listInsights(status ?? "pending", limit ?? 5); | |
| status: Type.Optional( | |
| Type.Union( | |
| [Type.Literal("pending"), Type.Literal("accepted"), Type.Literal("all")], | |
| { description: "Filter by status: pending, accepted, or all" }, | |
| ), | |
| ), | |
| limit: Type.Optional( | |
| Type.Integer({ minimum: 1, maximum: 50, description: "Max results to return (default: 5, max: 50)" }), | |
| ), | |
| }), | |
| async execute(_toolCallId: string, params: unknown): Promise<any> { | |
| const { status, limit } = params as { | |
| status?: "pending" | "accepted" | "all"; | |
| limit?: number; | |
| }; | |
| try { | |
| const safeStatus = status === "accepted" || status === "all" || status === "pending" ? status : "pending"; | |
| const safeLimit = Number.isInteger(limit) ? Math.min(50, Math.max(1, limit)) : 5; | |
| const result = await client.listInsights(safeStatus, safeLimit); |
| } | ||
| const text = items | ||
| .map((insight: any, i: number) => { | ||
| const conf = typeof insight.confidence === "number" ? ` (${Math.round(insight.confidence * 100)}%)` : ""; |
There was a problem hiding this comment.
typeof insight.confidence === "number" will also be true for NaN/Infinity, which can produce user-visible NaN%/Infinity% in output. Use a finite-number check (e.g., Number.isFinite) before formatting the percentage.
| const conf = typeof insight.confidence === "number" ? ` (${Math.round(insight.confidence * 100)}%)` : ""; | |
| const conf = Number.isFinite(insight.confidence) ? ` (${Math.round(insight.confidence * 100)}%)` : ""; |
| const text = items | ||
| .map((insight: any, i: number) => { | ||
| const conf = typeof insight.confidence === "number" ? ` (${Math.round(insight.confidence * 100)}%)` : ""; | ||
| const type = insight.insight_type ? ` [${insight.insight_type}]` : ""; | ||
| const statusTag = insight.status ? ` [${insight.status}]` : ""; | ||
| return `${i + 1}. ${insight.insight ?? insight.content ?? JSON.stringify(insight)}${conf}${type}${statusTag}`; |
There was a problem hiding this comment.
Falling back to JSON.stringify(insight) can emit very large blobs and may unintentionally surface fields that shouldn't be shown to the user. Prefer selecting a small, known-safe subset of fields (and truncating long text) for the fallback case to keep responses bounded and predictable.
| const text = items | |
| .map((insight: any, i: number) => { | |
| const conf = typeof insight.confidence === "number" ? ` (${Math.round(insight.confidence * 100)}%)` : ""; | |
| const type = insight.insight_type ? ` [${insight.insight_type}]` : ""; | |
| const statusTag = insight.status ? ` [${insight.status}]` : ""; | |
| return `${i + 1}. ${insight.insight ?? insight.content ?? JSON.stringify(insight)}${conf}${type}${statusTag}`; | |
| const truncateText = (value: unknown, max = 200): string | undefined => { | |
| if (typeof value !== "string") return undefined; | |
| const normalized = value.trim(); | |
| if (!normalized) return undefined; | |
| return normalized.length > max ? `${normalized.slice(0, max - 1)}…` : normalized; | |
| }; | |
| const formatInsightFallback = (insight: any): string => { | |
| const parts = [ | |
| truncateText(insight?.title, 80), | |
| truncateText(insight?.summary, 160), | |
| typeof insight?.id === "string" && insight.id.trim() ? `id=${insight.id.trim()}` : undefined, | |
| typeof insight?.insight_type === "string" && insight.insight_type.trim() | |
| ? `type=${insight.insight_type.trim()}` | |
| : undefined, | |
| typeof insight?.status === "string" && insight.status.trim() ? `status=${insight.status.trim()}` : undefined, | |
| typeof insight?.confidence === "number" ? `confidence=${Math.round(insight.confidence * 100)}%` : undefined, | |
| ].filter((part): part is string => Boolean(part)); | |
| return parts.length ? parts.join(" | ") : "Insight details unavailable"; | |
| }; | |
| const text = items | |
| .map((insight: any, i: number) => { | |
| const conf = typeof insight.confidence === "number" ? ` (${Math.round(insight.confidence * 100)}%)` : ""; | |
| const type = insight.insight_type ? ` [${insight.insight_type}]` : ""; | |
| const statusTag = insight.status ? ` [${insight.status}]` : ""; | |
| const displayText = | |
| truncateText(insight.insight, 280) ?? | |
| truncateText(insight.content, 280) ?? | |
| formatInsightFallback(insight); | |
| return `${i + 1}. ${displayText}${conf}${type}${statusTag}`; |
Summary
Testing
Summary by CodeRabbit
cortex_insightstool to retrieve and list insights with customizable status filtering and result limits.