Feature/memory pr (#894)

* feat: multi-turn context and conversation arc memory PR 2E - Section 2.9, 2.10: - Add multiTurnContext.ts with turn tracking and state preservation - Add conversationArc.ts with goal/decision/milestone tracking - Wire into query.ts after tool execution - Feature-flags: MULTI_TURN_CONTEXT, CONVERSATION_ARC - Add comprehensive tests (22 passing) * feat(cli): add /knowledge command to manage native memory - Add /knowledge enable <yes|no> to toggle Knowledge Graph learning\n- Add /knowledge clear to reset memory\n- Add persistent knowledgeGraphEnabled setting to global config\n- Integrated user setting into the query execution loop * feat(cli): add /knowledge command (stable local-jsx version) - Resolve conflicts between .ts and .tsx files\n- Align with LocalJSXCommandCall signature\n- Fix onDone and args errors * test(cli): fix knowledge command tests by properly isolating global config * fix(cli): make knowledge command defensive against undefined args and leaky tests * fix(cli): correct data source for entity count and fix test isolation * fix(cli): reinforce knowledge test by explicitly defining property on test config * fix(cli): explicitly define property in test config to avoid undefined in CI * fix(cli): make knowledge tests resistant to global config mocks in CI * chore(memory): surgical improvements from architectural audit - Fix: Implement entity deduplication in Knowledge Graph\n- Fix: Ensure fact extraction from user messages in query loop\n- Fix: Refine regexes for better quality learning (less noise) --------- Co-authored-by: LifeJiggy <Bloomtonjovish@gmail.com>
2026-04-25 01:19:41 +02:00
parent ff2a380723
commit 44f9cac70d
10 changed files with 247 additions and 84 deletions
--- a/src/utils/config.ts
+++ b/src/utils/config.ts
@@ -606,6 +606,9 @@ export type GlobalConfig = {
  // CURRENT_MIGRATION_VERSION, runMigrations() skips all sync migrations
  // (avoiding 11× saveGlobalConfig lock+re-read on every startup).
  migrationVersion?: number
+
+  // Knowledge Graph configuration
+  knowledgeGraphEnabled: boolean
 }

 /**
@@ -614,7 +617,7 @@ export type GlobalConfig = {
 * a factory gives fresh refs at zero clone cost.
 */
 function createDefaultGlobalConfig(): GlobalConfig {
-  return {
+  const config: GlobalConfig = {
    numStartups: 0,
    installMethod: undefined,
    autoUpdates: undefined,
@@ -653,7 +656,9 @@ function createDefaultGlobalConfig(): GlobalConfig {
    copyFullResponse: false,
    providerProfiles: [],
    openaiAdditionalModelOptionsCacheByProfile: {},
+    knowledgeGraphEnabled: true,
  }
+  return config
 }

 export const DEFAULT_GLOBAL_CONFIG: GlobalConfig = createDefaultGlobalConfig()
@@ -699,6 +704,7 @@ export const GLOBAL_CONFIG_KEYS = [
  'prStatusFooterEnabled',
  'remoteControlAtStartup',
  'remoteDialogSeen',
+  'knowledgeGraphEnabled',
 ] as const

 export type GlobalConfigKey = (typeof GLOBAL_CONFIG_KEYS)[number]
@@ -800,6 +806,7 @@ export function isPathTrusted(dir: string): boolean {
 const TEST_GLOBAL_CONFIG_FOR_TESTING: GlobalConfig = {
  ...DEFAULT_GLOBAL_CONFIG,
  autoUpdates: false,
+  knowledgeGraphEnabled: true,
 }
 const TEST_PROJECT_CONFIG_FOR_TESTING: ProjectConfig = {
  ...DEFAULT_PROJECT_CONFIG,
--- a/src/utils/conversationArc.test.ts
+++ b/src/utils/conversationArc.test.ts
@@ -86,7 +86,6 @@ describe('conversationArc', () => {
  })

  describe('resetArc', () => {
-
    it('returns existing arc or creates new', () => {
      const arc1 = getArc()
      const arc2 = getArc()
@@ -188,4 +187,4 @@ describe('conversationArc', () => {
      expect(stats?.decisionCount).toBe(1)
    })
  })
-})
+})
--- a/src/utils/conversationArc.ts
+++ b/src/utils/conversationArc.ts
@@ -119,25 +119,26 @@ function extractFactsAutomatically(content: string): void {
  const arc = getArc()
  if (!arc) return

-  // 1. Detect Environment Variables (KEY=VALUE)
-  const envMatches = content.matchAll(/(?:export\s+)?([A-Z_]+)=([^\s\n"']+)/g)
+  // 1. Detect Environment Variables (KEY=VALUE) - strictly uppercase keys
+  const envMatches = content.matchAll(/(?:export\s+)?([A-Z_]{3,})=([^\s\n"']+)/g)
  for (const match of envMatches) {
    addEntity('environment_variable', match[1], { value: match[2] })
  }

-  // 2. Detect Absolute Paths
+  // 2. Detect Absolute Paths - ensure it looks like a path and not a div or code
  const pathMatches = content.matchAll(/(\/(?:[\w.-]+\/)+[\w.-]+)/g)
  for (const match of pathMatches) {
    const path = match[1]
-    if (path.length > 5 && !path.includes('node_modules')) {
+    // Exclude common noise and ensure it's a long enough path
+    if (path.length > 8 && !path.includes('node_modules') && !path.includes('://')) {
      addEntity('path', path, { type: 'absolute' })
    }
  }

-  // 3. Detect Versions (v1.2.3 or version 1.2.3)
+  // 3. Detect Versions - require vX.Y.Z or version X.Y.Z
  const versionMatches = content.matchAll(/(?:v|version\s+)(\d+\.\d+(?:\.\d+)?)/gi)
  for (const match of versionMatches) {
-    addEntity('version', match[0], { semver: match[1] })
+    addEntity('version', match[0].toLowerCase(), { semver: match[1] })
  }

  // 4. Detect Hostnames/URLs
@@ -145,7 +146,9 @@ function extractFactsAutomatically(content: string): void {
  for (const match of urlMatches) {
    try {
      const url = new URL(match[1])
-      addEntity('endpoint', url.hostname, { url: url.toString() })
+      if (url.hostname.includes('.')) {
+        addEntity('endpoint', url.hostname, { url: url.toString() })
+      }
    } catch {
      // Ignore invalid URLs
    }
@@ -262,6 +265,17 @@ export function addEntity(
  const arc = getArc()
  if (!arc) throw new Error('Arc not initialized')

+  // Check for existing entity to avoid duplicates (Deduplication Logic)
+  const existingEntity = Object.values(arc.knowledgeGraph.entities).find(
+    e => e.type === type && e.name === name,
+  )
+
+  if (existingEntity) {
+    existingEntity.attributes = { ...existingEntity.attributes, ...attributes }
+    arc.lastUpdateTime = Date.now()
+    return existingEntity
+  }
+
  const id = `entity_${Date.now()}_${Math.random().toString(36).slice(2, 7)}`
  const entity: Entity = { id, type, name, attributes }

@@ -360,4 +374,4 @@ export function getArcStats() {
    milestoneCount: arc.milestones.length,
    durationMs: arc.lastUpdateTime - arc.startTime,
  }
-}
+}
--- a/src/utils/multiTurnContext.test.ts
+++ b/src/utils/multiTurnContext.test.ts
@@ -29,8 +29,8 @@ describe('multiTurnContext', () => {
    it('creates a new turn', () => {
      const turn = startNewTurn()
      expect(turn.turnId).toBeDefined()
+      expect(turn.startTime).toBeDefined()
      expect(turn.messages).toEqual([])
-      expect(turn.toolCalls).toEqual([])
    })

    it('tracks turn count', () => {
@@ -44,33 +44,34 @@ describe('multiTurnContext', () => {
    it('adds message to current turn', () => {
      startNewTurn()
      addMessageToTurn(createMessage('user', 'Hello'))
-
-      const turn = getCurrentTurn()
-      expect(turn?.messages.length).toBe(1)
+      expect(getCurrentTurn()?.messages.length).toBe(1)
    })

    it('creates turn if none exists', () => {
      addMessageToTurn(createMessage('user', 'Hello'))
-      expect(getCurrentTurn()).not.toBeNull()
+      expect(getCurrentTurn()).toBeDefined()
+      expect(getCurrentTurn()?.messages.length).toBe(1)
    })
  })

  describe('addToolCallToTurn', () => {
    it('adds tool call to turn', () => {
      startNewTurn()
-      addToolCallToTurn({ id: 'tool1', name: 'read', input: { file: 'test' }, timestamp: Date.now() })
-
-      const turn = getCurrentTurn()
-      expect(turn?.toolCalls.length).toBe(1)
-      expect(turn?.toolCalls[0].name).toBe('read')
+      addToolCallToTurn({
+        id: 'call_1',
+        name: 'test_tool',
+        input: {},
+        timestamp: Date.now(),
+      })
+      expect(getCurrentTurn()?.toolCalls.length).toBe(1)
    })
  })

  describe('state management', () => {
    it('sets and gets turn state', () => {
      startNewTurn()
-      setTurnState('key1', 'value1')
-      expect(getTurnState<string>('key1')).toBe('value1')
+      setTurnState('key', 'value')
+      expect(getTurnState('key')).toBe('value')
    })

    it('returns undefined for unknown keys', () => {
@@ -83,29 +84,26 @@ describe('multiTurnContext', () => {
    it('returns turn history', () => {
      startNewTurn()
      startNewTurn()
-
-      const history = getTurnHistory()
-      expect(history.length).toBe(2)
+      expect(getTurnHistory().length).toBe(2)
    })
  })

  describe('getRecentTurns', () => {
    it('returns recent turns', () => {
-      for (let i = 0; i < 5; i++) startNewTurn()
-
-      const recent = getRecentTurns(3)
-      expect(recent.length).toBe(3)
+      startNewTurn()
+      startNewTurn()
+      startNewTurn()
+      expect(getRecentTurns(2).length).toBe(2)
    })
  })

  describe('getMultiTurnStats', () => {
    it('returns statistics', () => {
      startNewTurn()
-      addMessageToTurn(createMessage('user', 'Test'))
-
+      addMessageToTurn(createMessage('user', 'Hello'))
      const stats = getMultiTurnStats()
      expect(stats.totalTurns).toBe(1)
-      expect(stats.currentTurnActive).toBe(true)
+      expect(stats.totalTokens).toBeGreaterThan(0)
    })
  })

@@ -120,15 +118,15 @@ describe('multiTurnContext', () => {
    it('respects the maxTurns option', () => {
      // Create a tracker with a very small maxTurns
      createMultiTurnTracker({ maxTurns: 2 })
-
+      
      startNewTurn() // turn 1
      startNewTurn() // turn 2
      startNewTurn() // turn 3 - should drop turn 1
-
+      
      const history = getTurnHistory()
      expect(history.length).toBe(2)
      // The first remaining turn should be the 2nd one created
      expect(history[0].turnId).toContain('turn_2')
    })
-    })
-    })
+  })
+})
--- a/src/utils/multiTurnContext.ts
+++ b/src/utils/multiTurnContext.ts
@@ -12,19 +12,16 @@ export interface TurnContext {
  turnId: string
  startTime: number
  messages: Message[]
-  toolCalls: ToolCallInfo[]
+  toolCalls: Array<{
+    id: string
+    name: string
+    input: Record<string, unknown>
+    timestamp: number
+  }>
  state: Map<string, unknown>
  tokens: number
 }

-export interface ToolCallInfo {
-  id: string
-  name: string
-  input: Record<string, unknown>
-  result?: string
-  timestamp: number
-}
-
 export interface MultiTurnOptions {
  maxTurns?: number
  maxTokensPerTurn?: number
@@ -33,7 +30,7 @@ export interface MultiTurnOptions {

 const DEFAULT_OPTIONS: Required<MultiTurnOptions> = {
  maxTurns: 10,
-  maxTokensPerTurn: 5000,
+  maxTokensPerTurn: 50000,
  preserveState: true,
 }

@@ -67,58 +64,45 @@ export function getCurrentTurn(): TurnContext | null {
 }

 export function addMessageToTurn(message: Message): void {
-  if (!currentTurn) {
-    currentTurn = startNewTurn()
-  }
-
-  const content = typeof message.message?.content === 'string'
-    ? message.message.content
-    : JSON.stringify(message.message?.content)
-
-  currentTurn.messages.push(message)
-  currentTurn.tokens += roughTokenCountEstimation(content)
+  const turn = currentTurn || startNewTurn()
+  turn.messages.push(message)
+  
+  // Update token estimate
+  const content = typeof message.message.content === 'string' 
+    ? message.message.content 
+    : JSON.stringify(message.message.content)
+  turn.tokens += roughTokenCountEstimation(content)
 }

-export function addToolCallToTurn(toolCall: ToolCallInfo): void {
-  if (!currentTurn) {
-    currentTurn = startNewTurn()
-  }
-
-  currentTurn.toolCalls.push(toolCall)
+export function addToolCallToTurn(call: TurnContext['toolCalls'][0]): void {
+  const turn = currentTurn || startNewTurn()
+  turn.toolCalls.push(call)
 }

 export function setTurnState(key: string, value: unknown): void {
-  if (!currentTurn) return
-  currentTurn.state.set(key, value)
+  const turn = currentTurn || startNewTurn()
+  turn.state.set(key, value)
 }

 export function getTurnState<T>(key: string): T | undefined {
-  if (!currentTurn) return undefined
-  return currentTurn.state.get(key) as T | undefined
+  return currentTurn?.state.get(key) as T
 }

 export function getTurnHistory(): TurnContext[] {
  return turnHistory
 }

-export function getRecentTurns(count: number): TurnContext[] {
-  return turnHistory.slice(-count)
-}
-
-export function getTurnById(turnId: string): TurnContext | undefined {
-  return turnHistory.find(t => t.turnId === turnId)
-}
-
-export function getCrossTurnContext(key: string): unknown[] {
-  return turnHistory.map(t => t.state.get(key)).filter(v => v !== undefined)
+export function getRecentTurns(n: number): TurnContext[] {
+  return turnHistory.slice(-n)
 }

 export function getMultiTurnStats() {
  return {
    totalTurns: turnHistory.length,
-    currentTurnActive: currentTurn !== null,
-    totalTokens: turnHistory.reduce((sum, t) => sum + t.tokens, 0),
-    totalToolCalls: turnHistory.reduce((sum, t) => sum + t.toolCalls.length, 0),
+    totalTokens: turnHistory.reduce((acc, t) => acc + t.tokens, 0),
+    avgTokensPerTurn: turnHistory.length > 0 
+      ? Math.round(turnHistory.reduce((acc, t) => acc + t.tokens, 0) / turnHistory.length) 
+      : 0,
  }
 }

@@ -146,4 +130,4 @@ export function createMultiTurnTracker(options: MultiTurnOptions = {}) {
    getStats: getMultiTurnStats,
    reset: resetMultiTurnState,
  }
-}
+}