-
Notifications
You must be signed in to change notification settings - Fork 2
Description
Context
PR #247 (insights CLI command) introduced ~170 lines of inline duplicate DB helper functions in cli/src/commands/insights.ts because the CLI cannot import from server (circular dependency: server → CLI). This refactor moves the canonical versions from server/src/llm/ to cli/src/analysis/, making the server files thin re-exports.
This follows the exact pattern of PR #238, which moved 9 prompt modules from server/src/llm/ to cli/src/analysis/.
TA-Approved Plan
TA has already reviewed and approved this plan. Dev must follow these specifications exactly.
Files Moving
server/src/llm/analysis-usage-db.ts → cli/src/analysis/analysis-usage-db.ts
SaveAnalysisUsageDatainterface (addsession_message_count?: number— TA adj. docs: update auth references for Supabase migration #5)AnalysisUsageRowinterfacesaveAnalysisUsage()— writes toanalysis_usagetable (preserve ON CONFLICT DO UPDATE, preservesession_message_countwrite)getSessionAnalysisUsage()— reads fromanalysis_usagetable (TA adj. feat: overhaul agent suite, hooks, and CLI types #3: move for table cohesion)
server/src/llm/analysis-db.ts → cli/src/analysis/analysis-db.ts
ANALYSIS_VERSIONconstant — export + accept as parameter with default (TA adj. chore: rebrand CLI from claudeinsight to code-insights #2)InsightRowinterfaceSessionDatainterface — keep separate fromSessionRow(TA adj. docs: clean up stale NextAuth/Prisma references #6: different contracts for different layers)DeleteOptionsinterface — keepincludeOnlyTypes(TA adj. docs: update auth references for Supabase migration #5: server uses it)convertToInsightRows()— converge divergences: add?? []guards for decisions/learnings (CLI version is safer)convertPromptQualityToInsightRow()— rename toconvertPQToInsightRowor keep both; align on one namesaveInsightsToDb()— add early-return guardif (insights.length === 0) returnfrom CLI versiondeleteSessionInsights()— keepincludeOnlyTypesparam from server, plus existingexcludeTypes/excludeIdssaveFacetsToDb()— add?? nullguards forworkflow_patternandcourse_correction_reason(CLI version is safer); acceptanalysisVersionfrom server signature (TA adj. chore: rebrand CLI from claudeinsight to code-insights #2 applies here too)- Preserve all
console.warnmonitors:[pattern-monitor](TA adj. Feature/agents hooks overhaul #4)
After the Move
- Server re-exports: Both server files become thin re-exports via
@code-insights/cli/analysis/* - Delete inline duplicates: Remove ~170 lines from
cli/src/commands/insights.ts(lines 36–212) - Replace with imports: Add imports from
../analysis/analysis-db.jsand../analysis/analysis-usage-db.js - CLI package.json: Add two new exports:
"./analysis/analysis-db": "./dist/analysis/analysis-db.js""./analysis/analysis-usage-db": "./dist/analysis/analysis-usage-db.js"
Key Divergences to Converge
| Behavior | Server version | CLI version | Resolution |
|---|---|---|---|
saveInsightsToDb early return |
No | if (insights.length === 0) return |
Keep CLI guard |
decisions ?? [] / learnings ?? [] |
No ?? |
Has ?? [] |
Keep CLI ?? [] |
saveFacetsToDb null guards |
No ?? null |
Has ?? null |
Keep CLI guards |
DeleteOptions.includeOnlyTypes |
Yes | No | Keep — server uses it |
SaveAnalysisUsageData.session_message_count |
No | Yes (inline only) | Add to interface |
ANALYSIS_VERSION in saveFacetsToDb |
Parameter | Hardcoded const | Parameter with default |
Scope Tags
- NOT VISUAL (no UI changes)
- NOT API_CHANGE (server re-exports preserve all public APIs)
- NOT NEW_DEPS
- Tier 1 verification only:
pnpm build+cd cli && pnpm test+cd server && pnpm test
TDD Requirement
Tests are required. Write or update tests in cli/src/analysis/__tests__/ for:
saveInsightsToDb— happy path, empty input early returndeleteSessionInsights— excludeTypes, includeOnlyTypes, excludeIdssaveFacetsToDb— with and without optional fields (null guards)saveAnalysisUsage— ON CONFLICT upsert behavior, session_message_count writegetSessionAnalysisUsage— returns empty array for unknown sessionconvertToInsightRows— decisions/learnings with missing confidence (boundary)convertPQToInsightRow— category normalization at write time
File Pointers
| File | Role |
|---|---|
server/src/llm/analysis-db.ts |
Source (canonical DB helpers) |
server/src/llm/analysis-usage-db.ts |
Source (canonical usage helpers) |
cli/src/commands/insights.ts |
Contains ~170 lines of inline duplicates to delete |
cli/src/analysis/ |
Target directory (existing PR #238 modules live here) |
cli/package.json |
Needs 2 new exports |
cli/src/analysis/__tests__/ |
TDD test target |
Verification
# From repo root
pnpm build # Zero errors required
# Per-package tests
cd cli && pnpm test # All tests pass
cd server && pnpm test # All tests passNo PR until all three pass.
References
- PR feat(cli): insights command with --native, --hook, --force modes (Issue #240) #247:
insightsCLI command (introduced the inline duplicates) - PR refactor: migrate prompt modules from server to CLI (native analysis foundation) #238: precedent refactor (moved 9 prompt modules server → CLI)
- Worktree:
/Users/melagiri/Workspace/codeInsights/code-insights-move-analysis-db-helpers-to-cli/ - Branch:
feature/move-analysis-db-helpers-to-cli