fix(insights): add queued status and fix action lifecycle semantics#52
fix(insights): add queued status and fix action lifecycle semantics#52serrrfirat wants to merge 7 commits into
Conversation
… tests - Fix similarity() bias: use Math.max denominator to prevent inflated scores - Fix mutation of shared ranked objects by cloning before modifying keep/suppressionReasons - Use stable rank-based identity keys instead of fragile rank:topic strings - Extract parseInsightBiasMode to config.ts (was duplicated in api-server + index) - Extract recordRankingTelemetry helper to insight-ranker.ts (was duplicated 50+ lines in cli + daemon) - Filter telemetry events at SQL level in getInsightQualityMetrics instead of loading all events - Add 3 new tests: consistency bias, exploration guarantee swap, feedback accumulation - Add JSDoc for scoring formula and document normalizeActions .slice(0,4) cap Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…etry tests - Add skipSelectedForSurface option to recordRankingTelemetry so the daemon does not double-emit insight_selected_for_surface (it records its own after saveInsight with the real entityId) - Add 2 tests for recordRankingTelemetry: correct event counts for mixed kept/suppressed results, and skipSelectedForSurface behavior - Add inline comments noting shallow clone safety for nested read-only objects Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ass event classification Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Insight status was incorrectly set to 'acted' at job queue time. If the
job later failed, the insight was permanently mislabeled. This introduces
a 'queued' intermediate status and cascades the final status from
execution results. Also fixes the API to return proper HTTP status codes
(202 Accepted, 404, 422) instead of 200 with { success: false }.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @serrrfirat, 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 robustness and clarity of the insight action lifecycle. By introducing a 'queued' status and refining the distinction between queuing an action and marking it as acted, the system now accurately reflects the state of an insight's associated job. Furthermore, the changes ensure that insight statuses are automatically updated based on the success or failure of their corresponding executions, and API responses for action requests are more precise, leading to a more reliable and understandable user experience. Highlights
Changelog
Activity
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
|
![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
The pull request successfully introduces the 'queued' status to the insight lifecycle and improves action semantics by cascading execution results to the parent insight. It also refines API response codes to follow RESTful conventions. However, there are logic discrepancies in the similarity calculation that may degrade personalization accuracy, and redundant status updates in the action executor that bypass the newly implemented guards in the database layer.
| // No content at all | ||
| const errorMsg = 'No output generated'; | ||
| updateExecutionStatus(db, executionId, 'failed', errorMsg, 'unknown'); | ||
| updateInsightStatus(db, insight.id!, 'viewed'); |
There was a problem hiding this comment.
This call to updateInsightStatus is now redundant because updateExecutionStatus (called on the previous line) already handles the cascade to the parent insight. Furthermore, calling it explicitly here bypasses the guard logic implemented in synapse-db.ts (which only reverts to 'viewed' if the status is still 'queued'), potentially leading to inconsistent states or duplicate telemetry events.
| const setB = new Set(b); | ||
| const overlap = a.filter(token => setB.has(token)).length; | ||
| const denominator = Math.max(1, Math.min(a.length, b.length)); | ||
| const denominator = Math.max(1, Math.max(a.length, b.length)); |
There was a problem hiding this comment.
Changing the denominator from Math.min to Math.max significantly alters the similarity heuristic. Using Math.max makes the score much more conservative, especially when comparing sets of different lengths (e.g., a short topic vs. a long description). This inconsistency with the deduplication logic in synapse-db.ts (which still uses Math.min) may cause the ranker to fail to identify related topics, leading to poor personalization or duplicate insights being surfaced.
| const denominator = Math.max(1, Math.max(a.length, b.length)); | |
| const denominator = Math.max(1, Math.min(a.length, b.length)); |
| } | ||
|
|
||
| updateExecutionStatus(db, executionId, 'completed', undefined, 'unknown'); | ||
| updateInsightStatus(db, insight.id!, 'acted'); |
There was a problem hiding this comment.
Similar to the failure case, this explicit update to 'acted' is redundant as it is now handled automatically by the updateExecutionStatus(..., 'completed', ...) call on line 629.
| }); | ||
|
|
||
| // Cascade: mark the parent insight as 'acted' | ||
| const execRow = db.prepare(`SELECT insight_id FROM action_executions WHERE id = ?`).get(executionId) as { insight_id: number } | null; |
There was a problem hiding this comment.
The query to fetch insight_id is repeated at line 3049. Consider refactoring this into a single variable at the beginning of the function when the status is terminal (completed, failed, or cancelled) to avoid redundant database lookups.
serrrfirat
left a comment
serrrfirat
left a comment
There was a problem hiding this comment.
Review: REQUEST CHANGES
Blocking Issues
-
HIGH: Double-write on insight status — Every
updateExecutionStatus(db, executionId, 'completed')cascades an internalupdateInsightStatuscall, then the caller ALSO callsupdateInsightStatusdirectly. Result: 2x UPDATE queries and 2x telemetry events per execution. Fix: remove explicitupdateInsightStatuscalls fromaction-executor.ts(keep cascade insynapse-db.ts). -
HIGH: Success cascade lacks guard —
updateExecutionStatus('completed')unconditionally sets insight to'acted', but the failure path IS guarded byif (insightRow?.status === 'queued'). If a user dismisses an insight while a job runs, the success cascade overwrites'dismissed'with'acted'.
Non-Blocking Issues
similarity()denominator change (same as PR #50/#51)- No transaction wrapping around status + telemetry multi-statement operations
createJobuses a different DB connection than the caller'sdb- API error matching uses fragile string comparison (
result.error === 'Action or insight not found') - Non-null assertion
insight.id!used without guard
…Jaccard - Remove redundant updateInsightStatus calls from action-executor.ts since updateExecutionStatus already cascades insight status changes - Guard the success cascade in updateExecutionStatus so 'completed' does not overwrite 'dismissed' with 'acted' - Extract insight_id lookup to a single query at the top of the terminal-status block, eliminating duplicate DB queries - Fix similarity() denominator to use true Jaccard (|A|+|B|-|A∩B|) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughIntroduces an insight action "queued" lifecycle: jobs now queue actions ( Changes
Sequence DiagramsequenceDiagram
actor Client
participant API as API Server
participant AS as Action Service
participant AE as Action Executor
participant DB as Database
participant Telemetry
Client->>API: POST /api/insights/:id/act
API->>AS: createActionJob(insightId, action)
AS->>DB: queueInsightAction(db, insightId, actionLabel, source?)
DB->>DB: SET insight.status = 'queued', record selected_action
DB->>Telemetry: insight_action_queued
AS-->>API: { success: true }
API-->>Client: 202 Accepted (queued)
rect rgba(100, 200, 100, 0.5)
Note over AE: Async Execution — Success path
AE->>AE: Execute action
AE-->>AE: Success
AE->>DB: markInsightActed(db, insightId, actionLabel, source?)
DB->>DB: SET insight.status = 'acted'
DB->>Telemetry: insight_acted
end
rect rgba(200, 100, 100, 0.5)
Note over AE: Async Execution — Failure path
AE->>AE: Execute action
AE-->>AE: Failure
AE->>DB: updateInsightStatus(db, insightId, 'viewed') if status == 'queued'
DB->>DB: SET insight.status = 'viewed'
DB->>Telemetry: insight_status_reverted
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/synapse-db.ts (1)
3037-3057:⚠️ Potential issue | 🟡 MinorAdd guard against reverting insight while other executions are in-flight.
If multiple executions per insight are possible, a failed/cancelled execution could revert the insight to
viewedwhile another execution is stillpending/running. Add a check for other in-flight executions before downgrading the insight status:Suggested guard
if (execRow?.insight_id) { const insightRow = db.prepare(`SELECT status FROM insights WHERE id = ?`).get(execRow.insight_id) as { status: string } | null; if (insightRow?.status === 'queued') { + const otherInFlight = db.prepare( + `SELECT 1 FROM action_executions WHERE insight_id = ? AND status IN ('pending', 'running') AND id != ? LIMIT 1` + ).get(execRow.insight_id, executionId); + if (!otherInFlight) { updateInsightStatus(db, execRow.insight_id, 'viewed', source); + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synapse-db.ts` around lines 3037 - 3057, Before downgrading the related insight in the failed/cancelled branch, query the action_executions table for other in-flight executions for the same execRow.insight_id (excluding the current executionId) and only call updateInsightStatus(db, execRow.insight_id, 'viewed', source) if that count is zero; specifically, use a prepared statement (e.g., SELECT COUNT(1) AS cnt FROM action_executions WHERE insight_id = ? AND id != ? AND status IN ('queued','pending','running')) and check cnt === 0 before reverting the insight status.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.ts`:
- Around line 1645-1647: The total currently sums counts.new, counts.viewed,
counts.queued, counts.acted, and counts.dismissed but the console.log summary
only lists new/viewed/queued/acted, causing a mismatch; update the logging
statement in the same block (where total is calculated and the console.log is
emitted) to include counts.dismissed in the parentheses (or alternatively remove
counts.dismissed from the total calculation) so the printed breakdown matches
the computed total—look for the total variable and the console.log call that
formats the summary and add the dismissed count label (counts.dismissed || 0) to
the message.
In `@src/insight-ranker.ts`:
- Around line 515-569: The telemetry currently uses ranked.keep (from
ranking.ranked) which can be false for items later promoted by exploration;
change recordRankingTelemetry to determine "kept" from the final selection set
(ranking.selected) instead: compute a boolean like const isKept =
ranking.selected.some(s => s.id === ranked.id) (or compare the same unique
identifier used elsewhere) and use isKept in the metadata (replace kept:
ranked.keep) and in the conditionals that emit insight_selected_for_surface vs
insight_suppressed (respecting options.skipSelectedForSurface). Update all
places that reference ranked.keep for telemetry to use this derived isKept value
so promoted candidates are correctly recorded as kept.
In `@src/synapse-db.ts`:
- Around line 1344-1364: queueInsightAction currently emits an eventType of
'insight_action_queued' which will break existing metrics that expect
'insight_action_selected'; to fix, update queueInsightAction to also emit the
legacy signal by calling recordTelemetryEvent a second time with eventType
'insight_action_selected' (same source, entityType 'insight', entityId
insightId, and metadata { actionLabel }) so both new and existing metrics
continue to work.
---
Outside diff comments:
In `@src/synapse-db.ts`:
- Around line 3037-3057: Before downgrading the related insight in the
failed/cancelled branch, query the action_executions table for other in-flight
executions for the same execRow.insight_id (excluding the current executionId)
and only call updateInsightStatus(db, execRow.insight_id, 'viewed', source) if
that count is zero; specifically, use a prepared statement (e.g., SELECT
COUNT(1) AS cnt FROM action_executions WHERE insight_id = ? AND id != ? AND
status IN ('queued','pending','running')) and check cnt === 0 before reverting
the insight status.
| const total = (counts.new || 0) + (counts.viewed || 0) + (counts.queued || 0) + (counts.acted || 0) + (counts.dismissed || 0); | ||
| if (total > insights.length) { | ||
| console.log(chalk.gray(`Total: ${total} insights (${counts.new || 0} new, ${counts.viewed || 0} viewed, ${counts.acted || 0} acted on)`)); | ||
| console.log(chalk.gray(`Total: ${total} insights (${counts.new || 0} new, ${counts.viewed || 0} viewed, ${counts.queued || 0} queued, ${counts.acted || 0} acted on)`)); |
There was a problem hiding this comment.
Total includes dismissed insights but summary omits them.
The total count includes dismissed, but the label doesn’t, so the breakdown may not add up. Consider including dismissed in the summary (or exclude it from total).
🛠️ Suggested fix
- console.log(chalk.gray(`Total: ${total} insights (${counts.new || 0} new, ${counts.viewed || 0} viewed, ${counts.queued || 0} queued, ${counts.acted || 0} acted on)`));
+ console.log(chalk.gray(`Total: ${total} insights (${counts.new || 0} new, ${counts.viewed || 0} viewed, ${counts.queued || 0} queued, ${counts.acted || 0} acted on, ${counts.dismissed || 0} dismissed)`));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const total = (counts.new || 0) + (counts.viewed || 0) + (counts.queued || 0) + (counts.acted || 0) + (counts.dismissed || 0); | |
| if (total > insights.length) { | |
| console.log(chalk.gray(`Total: ${total} insights (${counts.new || 0} new, ${counts.viewed || 0} viewed, ${counts.acted || 0} acted on)`)); | |
| console.log(chalk.gray(`Total: ${total} insights (${counts.new || 0} new, ${counts.viewed || 0} viewed, ${counts.queued || 0} queued, ${counts.acted || 0} acted on)`)); | |
| const total = (counts.new || 0) + (counts.viewed || 0) + (counts.queued || 0) + (counts.acted || 0) + (counts.dismissed || 0); | |
| if (total > insights.length) { | |
| console.log(chalk.gray(`Total: ${total} insights (${counts.new || 0} new, ${counts.viewed || 0} viewed, ${counts.queued || 0} queued, ${counts.acted || 0} acted on, ${counts.dismissed || 0} dismissed)`)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.ts` around lines 1645 - 1647, The total currently sums counts.new,
counts.viewed, counts.queued, counts.acted, and counts.dismissed but the
console.log summary only lists new/viewed/queued/acted, causing a mismatch;
update the logging statement in the same block (where total is calculated and
the console.log is emitted) to include counts.dismissed in the parentheses (or
alternatively remove counts.dismissed from the total calculation) so the printed
breakdown matches the computed total—look for the total variable and the
console.log call that formats the summary and add the dismissed count label
(counts.dismissed || 0) to the message.
| export function recordRankingTelemetry( | ||
| db: Parameters<typeof recordTelemetryEvent>[0], | ||
| ranking: InsightRankingResult, | ||
| options: { | ||
| source: TelemetrySource; | ||
| biasMode: string; | ||
| provider?: string; | ||
| skipSelectedForSurface?: boolean; | ||
| } | ||
| ): void { | ||
| for (const ranked of ranking.ranked) { | ||
| const commonMetadata = { | ||
| ...(options.provider ? { provider: options.provider } : {}), | ||
| biasMode: options.biasMode, | ||
| score: ranked.score, | ||
| rank: ranked.rank, | ||
| isNovel: ranked.isNovel, | ||
| topicSimilarity: ranked.topicSimilarity, | ||
| }; | ||
|
|
||
| recordTelemetryEvent(db, { | ||
| eventType: 'insight_ranked', | ||
| source: options.source, | ||
| entityType: 'insight_candidate', | ||
| metadata: { | ||
| ...commonMetadata, | ||
| kept: ranked.keep, | ||
| memoryAlignment: ranked.features.memoryAlignment, | ||
| feedbackAdjustment: ranked.features.feedbackAdjustment, | ||
| noveltyPenalty: ranked.features.noveltyPenalty, | ||
| actionability: ranked.features.actionability, | ||
| matchedFeedbackCount: ranked.matchedFeedbackCount, | ||
| matchedMemoryCount: ranked.matchedMemoryCount, | ||
| suppressionReasons: ranked.suppressionReasons, | ||
| }, | ||
| }); | ||
|
|
||
| if (ranked.keep && !options.skipSelectedForSurface) { | ||
| recordTelemetryEvent(db, { | ||
| eventType: 'insight_selected_for_surface', | ||
| source: options.source, | ||
| entityType: 'insight_candidate', | ||
| metadata: commonMetadata, | ||
| }); | ||
| } else if (!ranked.keep) { | ||
| recordTelemetryEvent(db, { | ||
| eventType: 'insight_suppressed', | ||
| source: options.source, | ||
| entityType: 'insight_candidate', | ||
| metadata: { | ||
| ...commonMetadata, | ||
| suppressionReasons: ranked.suppressionReasons, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Telemetry “kept” should reflect final selection, not threshold-only.
recordRankingTelemetry uses ranked.keep, but exploration promotion can select candidates whose keep is still false in ranking.ranked. That mislabels promoted candidates as suppressed and skews selection metrics. Consider deriving “kept” from ranking.selected.
🛠️ Suggested fix
export function recordRankingTelemetry(
db: Parameters<typeof recordTelemetryEvent>[0],
ranking: InsightRankingResult,
options: {
source: TelemetrySource;
biasMode: string;
provider?: string;
skipSelectedForSurface?: boolean;
}
): void {
+ const selectedRanks = new Set(ranking.selected.map(entry => entry.rank));
for (const ranked of ranking.ranked) {
+ const kept = selectedRanks.has(ranked.rank);
const commonMetadata = {
...(options.provider ? { provider: options.provider } : {}),
biasMode: options.biasMode,
score: ranked.score,
rank: ranked.rank,
isNovel: ranked.isNovel,
topicSimilarity: ranked.topicSimilarity,
};
recordTelemetryEvent(db, {
eventType: 'insight_ranked',
source: options.source,
entityType: 'insight_candidate',
metadata: {
...commonMetadata,
- kept: ranked.keep,
+ kept,
memoryAlignment: ranked.features.memoryAlignment,
feedbackAdjustment: ranked.features.feedbackAdjustment,
noveltyPenalty: ranked.features.noveltyPenalty,
actionability: ranked.features.actionability,
matchedFeedbackCount: ranked.matchedFeedbackCount,
matchedMemoryCount: ranked.matchedMemoryCount,
suppressionReasons: ranked.suppressionReasons,
},
});
- if (ranked.keep && !options.skipSelectedForSurface) {
+ if (kept && !options.skipSelectedForSurface) {
recordTelemetryEvent(db, {
eventType: 'insight_selected_for_surface',
source: options.source,
entityType: 'insight_candidate',
metadata: commonMetadata,
});
- } else if (!ranked.keep) {
+ } else if (!kept) {
recordTelemetryEvent(db, {
eventType: 'insight_suppressed',
source: options.source,
entityType: 'insight_candidate',
metadata: {
...commonMetadata,
suppressionReasons: ranked.suppressionReasons,
},
});
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/insight-ranker.ts` around lines 515 - 569, The telemetry currently uses
ranked.keep (from ranking.ranked) which can be false for items later promoted by
exploration; change recordRankingTelemetry to determine "kept" from the final
selection set (ranking.selected) instead: compute a boolean like const isKept =
ranking.selected.some(s => s.id === ranked.id) (or compare the same unique
identifier used elsewhere) and use isKept in the metadata (replace kept:
ranked.keep) and in the conditionals that emit insight_selected_for_surface vs
insight_suppressed (respecting options.skipSelectedForSurface). Update all
places that reference ranked.keep for telemetry to use this derived isKept value
so promoted candidates are correctly recorded as kept.
| /** | ||
| * Record which action the user selected for an insight | ||
| * Queue an action for an insight — sets status to 'queued' and records selected_action. | ||
| * Used when an action job is created but not yet executed. | ||
| */ | ||
| export function selectInsightAction( | ||
| export function queueInsightAction( | ||
| db: Database, | ||
| insightId: number, | ||
| actionLabel: string, | ||
| source: TelemetrySource = 'unknown' | ||
| ): void { | ||
| const stmt = db.prepare(`UPDATE insights SET status = 'queued', selected_action = ? WHERE id = ?`); | ||
| stmt.run(actionLabel, insightId); | ||
|
|
||
| recordTelemetryEvent(db, { | ||
| eventType: 'insight_action_queued', | ||
| source, | ||
| entityType: 'insight', | ||
| entityId: insightId, | ||
| metadata: { actionLabel }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Telemetry signal change may break pilot metrics.
queueInsightAction now emits insight_action_queued, but pilot metrics still query insight_action_selected. That will undercount action selection milestones for the normal queue→execute path. Consider emitting the legacy event as well (or update the metrics to use insight_action_queued).
🛠️ Possible compatibility fix
recordTelemetryEvent(db, {
eventType: 'insight_action_queued',
source,
entityType: 'insight',
entityId: insightId,
metadata: { actionLabel },
});
+
+ // Backward-compatible signal for pilot metrics that track selection.
+ recordTelemetryEvent(db, {
+ eventType: 'insight_action_selected',
+ source,
+ entityType: 'insight',
+ entityId: insightId,
+ metadata: { actionLabel, queued: true },
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Record which action the user selected for an insight | |
| * Queue an action for an insight — sets status to 'queued' and records selected_action. | |
| * Used when an action job is created but not yet executed. | |
| */ | |
| export function selectInsightAction( | |
| export function queueInsightAction( | |
| db: Database, | |
| insightId: number, | |
| actionLabel: string, | |
| source: TelemetrySource = 'unknown' | |
| ): void { | |
| const stmt = db.prepare(`UPDATE insights SET status = 'queued', selected_action = ? WHERE id = ?`); | |
| stmt.run(actionLabel, insightId); | |
| recordTelemetryEvent(db, { | |
| eventType: 'insight_action_queued', | |
| source, | |
| entityType: 'insight', | |
| entityId: insightId, | |
| metadata: { actionLabel }, | |
| }); | |
| } | |
| /** | |
| * Queue an action for an insight — sets status to 'queued' and records selected_action. | |
| * Used when an action job is created but not yet executed. | |
| */ | |
| export function queueInsightAction( | |
| db: Database, | |
| insightId: number, | |
| actionLabel: string, | |
| source: TelemetrySource = 'unknown' | |
| ): void { | |
| const stmt = db.prepare(`UPDATE insights SET status = 'queued', selected_action = ? WHERE id = ?`); | |
| stmt.run(actionLabel, insightId); | |
| recordTelemetryEvent(db, { | |
| eventType: 'insight_action_queued', | |
| source, | |
| entityType: 'insight', | |
| entityId: insightId, | |
| metadata: { actionLabel }, | |
| }); | |
| // Backward-compatible signal for pilot metrics that track selection. | |
| recordTelemetryEvent(db, { | |
| eventType: 'insight_action_selected', | |
| source, | |
| entityType: 'insight', | |
| entityId: insightId, | |
| metadata: { actionLabel, queued: true }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synapse-db.ts` around lines 1344 - 1364, queueInsightAction currently
emits an eventType of 'insight_action_queued' which will break existing metrics
that expect 'insight_action_selected'; to fix, update queueInsightAction to also
emit the legacy signal by calling recordTelemetryEvent a second time with
eventType 'insight_action_selected' (same source, entityType 'insight', entityId
insightId, and metadata { actionLabel }) so both new and existing metrics
continue to work.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
'queued'intermediate status to the insight lifecycle (new→viewed→queued→acted/ back toviewed), preventing insights from being permanently mislabeled as'acted'when their action job failsselectInsightActionintoqueueInsightAction(job creation) andmarkInsightActed(bridge feedback for already-completed actions)updateExecutionStatusnow marks the parent insight'acted'on completion, or reverts to'viewed'on failure (guarded to not overwrite a successful execution)POST /api/insights/:id/actnow returns202 Acceptedon success,404for missing insights, and422for other errors — instead of200with{ success: false }Test plan
bunx tsc --noEmit— zero type errorsbun test— all 55 tests pass'queued'not'acted'202on success,404on missing insight,422on other errors'viewed'Closes #38
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests