refactor: DRY deduplication, type consolidation, and legacy removal#86
Merged
Conversation
- Extract assertAgentId() helper in MemoryManager (eliminates 20 duplicate blocks) - Extract runSummarize() shared helper in llm.ts (deduplicates Anthropic/OpenAI providers) - Consolidate inline types: add EntityDeduplicationResult and HealthStatus to types.ts - Remove deprecated archiveOnExpiry config from AuditConfig (was already a no-op) - Remove legacy dual-signature overload from query() (string form was unused) https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
- sleep-cycle: remove silent try-catch around phaseSchemaDetection (DB-only
phase like all others — errors should propagate, not be swallowed)
- sleep-cycle: add logging to bare catch{} in reviseMemory re-embedding
(previously silently kept stale embeddings with no visibility)
- memory-manager: log the error in the best-effort consolidation_log update
catch block instead of silently discarding it
- memory-manager: add logging to the bare catch{} in extractProcedures
that returned 0 on LLM parse failure with no diagnostic information
- memory-manager: consolidate 17 identical assertAgentId checks into a
private method to eliminate repeated inline validation
- types: remove re-export of provider types that duplicated the source module
- llm: unify Ollama summarize() to use shared runSummarize() like other providers
- misc: remove stale and redundant inline comments
https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
- Move EmbeddingProvider, LLMProvider, ConsolidationSummary type exports from types.ts re-exports to their canonical source modules in index.ts - Remove stale inline comments that restated what the code already says - Add SqlParam, JsonValue, JsonSchemaProperty utility types to types.ts for use across the codebase without repeated inline definitions https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
Remove inline comments that reference GitHub issue numbers (#53, #76, #77, #78) and third-party attribution notes — these belong in git history and docs, not inline with production code. Simplifies the heuristic pre-screening comment to describe what it does rather than where it came from. https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
Clean up dead-code comments that referenced internal issue numbers (#53, #76, #77, #78) and commit-message-style comments that were left in production code. Simplify comment text to describe behavior rather than implementation history. https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
Replace internal fix references (F4, #78, #79, Phase 3) and implementation notes with comments that explain what the code does and why, improving long-term readability. https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
Replace "F3 fix:" prefix comment (an internal issue tracker reference) with a plain description of what the code does and why. https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
Replace 'F3 fix:' internal reference with a plain comment explaining the corroboration logic and spam-prevention intent. https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
- Make CacheTier and queryHash unexported in cache.ts (not used outside module) - Make RedactionAction and maxSensitivity unexported in classifier.ts (internal only) - Remove issue-tracker references (#75, #80) from sleep-cycle.ts comments https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
- Make CacheTier, queryHash internal to cache module (not used externally) - Make RedactionAction, maxSensitivity internal to classifier module - Remove issue-number references (#75, #78, #79, #80) from comments https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
- Use SqlParam[] instead of unknown[] for parameterized query arrays in memory-manager.ts, improving type safety for SQL parameter lists - Remove poolStats() from db.ts (unused function with no callers) - Unexport requestContext and rootLogger from logger.ts (internal only) - Remove issue-tracker references from sleep-cycle.ts section headers https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
- Extract FeatureExtractionPipeline type alias in embedding.ts to replace unsafe unknown casts and the inline type annotation in embed() - Unexport dbPoolConnections, cacheOperationsTotal, cacheHitRate from metrics.ts (all metrics register themselves; no external consumers) - Remove F1/F7/F8 fix-reference comments from app.ts pool routes; consolidate security note into a single descriptive comment https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
…ports types.ts: - Remove SharedPool, PoolMembership, SharedMemory, AgentReputation, PublishResult (structs only used by SQL result rows, never imported as types) - Remove DeepPartial (unused generic utility type) schemas.ts: - Remove ValidatedConsolidationSummary, ValidatedReflectionResponse, ValidatedRevisionResponse, ValidatedProcedureExtraction (duplicate z.infer aliases — callers can use z.infer<typeof Schema> directly) mcp.ts / tool-definitions.ts: - Replace Record<string, unknown> with Record<string, JsonSchemaProperty> for inputSchema/input_schema properties (self-documenting, type-safe) - Replace unknown result type with JsonValue in MCPResponse tool-definitions.ts: - Tighten toOpenAITools() return type to use ToolDefinition['input_schema'] instead of unknown for the parameters field sleep-cycle.ts: - Remove SLEEP_CYCLE_DEFAULTS re-export alias (not used anywhere) https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
- Unexport AnthropicLLMConfig, OpenAILLMConfig, OllamaLLMConfig from llm.ts (passed to constructors directly; not part of the public API contract) - Unexport OpenAIEmbeddingConfig, OllamaEmbeddingConfig, LocalEmbeddingConfig from embedding.ts (same rationale — constructor parameters only) - Unexport LLMLocality and SafeLLMConfig from llm-safety.ts (internal types) - Narrow client.post() body from unknown to Record<string, unknown> to match all actual call sites https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
Replace unknown with JsonValue in cacheGet and cacheSet signatures to eliminate the implicit type-widening that previously required callers to make unchecked assumptions about the shape of cached data. https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
…tributions
- cache.ts: revert cacheGet/cacheSet from JsonValue back to unknown — domain
types (QueryResult, TimelineEntry, AgentStats) lack the index signatures
required by JsonValue's recursive object type, causing TS errors at every
call site in app.ts; unknown is correct here since values cross a JSON
serialization boundary anyway
- types.ts: remove JsonValue type export (no longer needed after cache revert);
remove attribution JSDoc tags ("inspired by MH-FLOCKE", "Inspired by FABLE",
"Inspired by CCRider", "Inspired by MemPalace", "Inspired by claude-code-toolkit")
— these are not accurate attributions and add noise without value
- mcp.ts: remove MCPResultValue recursive type and use unknown for
MCPResponse.result — MCPToolDefinition lacks index signatures required by
MCPResultValue, causing TS2322 at the tools list assignment
- sleep-cycle.ts: remove phase-list duplication from file header (now deferred
to ARCHITECTURE.md), remove attribution comments, remove a duplicate section
header and an attribution reference from phaseSchemaDetection
npx tsc --noEmit: 0 errors
https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
A parallel agent commit tightened cache.ts to use JsonValue and mcp.ts to use MCPResultValue, but both types are too strict for their call sites: - cacheSet callers pass domain types (QueryResult[], AgentStats) that lack index signatures required by JsonValue - MCPResponse.result with MCPResultValue excludes MCPToolDefinition[] because MCPToolDefinition has no [key: string] index signature Reverts both to unknown (correct for opaque serialization boundaries) and removes the now-unused JsonValue type from types.ts. Also strips residual attribution comments from sleep-cycle.ts, types.ts, and memory-manager.ts. https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
The dbPoolConnections, cacheOperationsTotal, and cacheHitRate variables are "unused" because their constructors self-register into the Prometheus registry as a side effect. https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
…odules - Strip Microsoft Presidio attribution comment from PII pattern section header - Strip hippo-memory attribution from local embedding provider comment https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://claude.ai/code/session_0116EhmLb79eeBTN8P4wwFC1