Repository navigation
refactor(types): comprehensive types module architecture refactoring - #194
Merged
murdore merged 1 commit intoOct 14, 2025
Merged
Conversation
WalkthroughThis PR centralizes and refactors TypeScript types across the codebase: moves inline/local types into src/lib/types/*, converts many interfaces to type aliases, adds new shared types (tools, utilities, content, providers), and updates imports in core modules. No runtime/control-flow changes were introduced. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/lib/core/baseProvider.ts(1 hunks)src/lib/core/conversationMemoryFactory.ts(1 hunks)src/lib/core/redisConversationMemoryManager.ts(1 hunks)src/lib/middleware/builtin/guardrails.ts(1 hunks)src/lib/neurolink.ts(1 hunks)src/lib/types/common.ts(4 hunks)src/lib/types/constants.ts(1 hunks)src/lib/types/index.ts(1 hunks)src/lib/types/mcpTypes.ts(9 hunks)src/lib/types/middleware.ts(1 hunks)src/lib/types/providers.ts(2 hunks)src/lib/types/sdkTypes.ts(1 hunks)src/lib/types/streamTypes.ts(1 hunks)src/lib/types/tools.ts(2 hunks)src/lib/types/utilities.ts(1 hunks)src/lib/utils/providerConfig.ts(1 hunks)src/lib/utils/timeout.ts(1 hunks)todos/refactor/07-types-module.md(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: RajuSudhar
PR: juspay/neurolink#173
File: src/lib/types/index.ts:20-33
Timestamp: 2025-09-17T17:57:36.381Z
Learning: RajuSudhar follows a phased refactor approach to avoid merge conflicts - first consolidating types in focused PRs, then addressing import path updates in dedicated refactor tasks like todos/refactor/07-types-module.md.
Learnt from: sinha-sahil
PR: juspay/neurolink#81
File: todos/refactor/01-global-imports.md:10-22
Timestamp: 2025-08-19T06:38:07.850Z
Learning: The team plans to update TypeScript's moduleResolution settings to support extensionless imports across the codebase, addressing potential NodeNext ESM runtime issues during the refactor process.
Learnt from: RajuSudhar
PR: juspay/neurolink#174
File: src/lib/mcp/contracts/mcpContract.ts:0-0
Timestamp: 2025-09-28T21:00:08.235Z
Learning: The src/lib/mcp/contracts/mcpContract.ts file was completely removed during the MCP types refactor in PR #174, with its types moved to centralized modules like src/lib/types/mcpTypes.ts and src/lib/types/index.ts.
Learnt from: RajuSudhar
PR: juspay/neurolink#174
File: src/lib/types/tools.ts:166-171
Timestamp: 2025-09-28T21:09:49.599Z
Learning: RajuSudhar prefers to defer type naming collision fixes to follow-up refactor phases when the primary focus is type consolidation and centralization.
📚 Learning: 2025-09-17T17:55:15.261Z
Learnt from: RajuSudhar
PR: juspay/neurolink#173
File: src/lib/index.ts:16-16
Timestamp: 2025-09-17T17:55:15.261Z
Learning: In src/lib/types/providers.ts, ProviderConfig was renamed to AIModelProviderConfig to deduplicate type names, as there was an existing ProviderConfig type that better suited the "ProviderConfig" name. This was an intentional breaking change for better type organization.
Applied to files:
src/lib/types/common.tssrc/lib/core/baseProvider.tssrc/lib/types/middleware.tssrc/lib/types/streamTypes.tssrc/lib/types/providers.tssrc/lib/utils/providerConfig.tssrc/lib/middleware/builtin/guardrails.tssrc/lib/types/tools.tssrc/lib/types/mcpTypes.tssrc/lib/core/conversationMemoryFactory.tstodos/refactor/07-types-module.md
📚 Learning: 2025-09-01T22:58:39.149Z
Learnt from: sudharsan-juspay
PR: juspay/neurolink#140
File: src/lib/core/types.ts:198-203
Timestamp: 2025-09-01T22:58:39.149Z
Learning: In src/lib/core/types.ts, StreamOptions (imported from streamTypes.js) and StreamingOptions are intentionally different types with different use cases. StreamingOptions is for unified AI requests with multiple provider configurations, while StreamOptions is for individual streaming operations.
Applied to files:
src/lib/types/common.tssrc/lib/types/streamTypes.tssrc/lib/types/providers.ts
📚 Learning: 2025-09-01T06:15:59.759Z
Learnt from: amreetkhuntia
PR: juspay/neurolink#133
File: src/lib/core/types.ts:208-210
Timestamp: 2025-09-01T06:15:59.759Z
Learning: The middleware?: MiddlewareFactoryOptions field is already present in both TextGenerationOptions and StreamOptions interfaces in the neurolink codebase.
Applied to files:
src/lib/core/baseProvider.ts
📚 Learning: 2025-09-28T21:00:08.235Z
Learnt from: RajuSudhar
PR: juspay/neurolink#174
File: src/lib/mcp/contracts/mcpContract.ts:0-0
Timestamp: 2025-09-28T21:00:08.235Z
Learning: The src/lib/mcp/contracts/mcpContract.ts file was completely removed during the MCP types refactor in PR #174, with its types moved to centralized modules like src/lib/types/mcpTypes.ts and src/lib/types/index.ts.
Applied to files:
src/lib/types/index.tssrc/lib/neurolink.tssrc/lib/types/mcpTypes.ts
📚 Learning: 2025-09-17T18:14:34.960Z
Learnt from: RajuSudhar
PR: juspay/neurolink#173
File: src/lib/types/index.ts:58-62
Timestamp: 2025-09-17T18:14:34.960Z
Learning: RajuSudhar explained that in the Neurolink codebase, there are multiple ProviderConfig types causing inconsistency. One existing ProviderConfig type better suited the "ProviderConfig" name, so they renamed the less-suitable one to AIModelProviderConfig to free up the name. Adding backward compatibility aliases would worsen naming inconsistency rather than help. The remaining duplicates will be systematically deduplicated in the 07-Types-Module.md TODO as part of their phased refactor approach.
Applied to files:
src/lib/types/providers.ts
📚 Learning: 2025-08-19T06:38:07.850Z
Learnt from: sinha-sahil
PR: juspay/neurolink#81
File: todos/refactor/01-global-imports.md:10-22
Timestamp: 2025-08-19T06:38:07.850Z
Learning: The team plans to update TypeScript's moduleResolution settings to support extensionless imports across the codebase, addressing potential NodeNext ESM runtime issues during the refactor process.
Applied to files:
src/lib/neurolink.ts
📚 Learning: 2025-09-17T17:57:36.381Z
Learnt from: RajuSudhar
PR: juspay/neurolink#173
File: src/lib/types/index.ts:20-33
Timestamp: 2025-09-17T17:57:36.381Z
Learning: RajuSudhar follows a phased refactor approach to avoid merge conflicts - first consolidating types in focused PRs, then addressing import path updates in dedicated refactor tasks like todos/refactor/07-types-module.md.
Applied to files:
todos/refactor/07-types-module.md
🧬 Code graph analysis (5)
src/lib/types/middleware.ts (1)
src/lib/middleware/index.ts (1)
LanguageModelV1Middleware(27-27)
src/lib/types/providers.ts (3)
src/lib/types/common.ts (1)
UnknownRecord(13-13)src/lib/types/index.ts (1)
UnknownRecord(38-38)src/lib/types/sdkTypes.ts (1)
UnknownRecord(164-164)
src/lib/types/utilities.ts (3)
src/lib/types/index.ts (11)
PerformanceMetric(171-171)UnknownRecord(38-38)PerformanceData(172-172)ProviderHealthResult(173-173)RetryConfig(26-26)ModelRoutingResult(174-174)LoggerConfig(175-175)RedisConnectionConfig(176-176)OptionsConversionResult(177-177)PromptRedactionConfig(178-178)MessageBuilderOptions(179-179)src/lib/types/common.ts (1)
UnknownRecord(13-13)src/lib/types/sdkTypes.ts (2)
UnknownRecord(164-164)RetryConfig(48-48)
src/lib/types/tools.ts (6)
src/lib/types/common.ts (5)
JsonValue(28-34)Result(55-59)ErrorInfo(45-50)UnknownRecord(13-13)JsonObject(36-38)src/lib/types/typeAliases.ts (5)
JsonValue(490-490)ToolContext(251-255)Result(276-278)JsonObject(490-490)ZodUnknownSchema(19-19)src/lib/sdk/toolRegistration.ts (2)
ToolContext(105-130)SimpleTool(136-164)src/lib/types/streamTypes.ts (2)
ToolResult(77-91)ToolCall(65-72)src/lib/mcp/factory.ts (1)
ToolResult(91-112)src/lib/types/mcpTypes.ts (2)
ToolMetadata(276-283)ToolRegistryEntry(256-264)
src/lib/types/mcpTypes.ts (5)
src/lib/index.ts (1)
MCPServerInfo(58-58)src/lib/types/index.ts (15)
MCPServerStatus(87-87)MCPDiscoveredServer(88-88)MCPConnectedServer(89-89)MCPToolInfo(90-90)ToolHandler(187-187)ToolDiscoveryResult(188-188)DiscoveredTool(189-189)MCPClientFactoryConfig(190-190)MCPClientInstance(191-191)ToolValidationResult(192-192)ExternalServerManagerConfig(193-193)ServerHealthResult(194-194)MCPContract(195-195)AIWorkflowToolConfig(196-196)AIAnalysisResult(197-197)src/lib/types/tools.ts (2)
ToolRegistryEntry(185-192)ToolMetadata(88-95)src/lib/mcp/toolDiscoveryService.ts (2)
ToolDiscoveryResult(28-46)ToolValidationResult(68-85)src/lib/mcp/mcpCircuitBreaker.ts (1)
CircuitBreakerConfig(18-36)
RajuSudhar
force-pushed
the
BZ-44578-types-module-refactor
branch
from
September 29, 2025 00:41
593bc7f to
e931ad6
Compare
murdore
force-pushed
the
BZ-44578-types-module-refactor
branch
from
September 30, 2025 16:15
e931ad6 to
72e0b76
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive refactoring of the TypeScript types module, transitioning from scattered inline type definitions to a centralized, modular architecture. The refactoring extracts type definitions from implementation files and consolidates them into dedicated type modules for improved maintainability and code organization.
Key changes:
- New type modules created: constants.ts, middleware.ts, utilities.ts for specialized type definitions
- Enhanced existing modules: mcpTypes.ts, tools.ts, providers.ts with additional extracted types
- Import standardization: Updated all consuming modules to use centralized type imports instead of inline definitions
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| todos/refactor/07-types-module.md | Updated completion status and documented successful Phase 1 completion |
| src/lib/utils/timeout.ts | Removed inline TimeoutConfig/TimeoutResult types, now imported from utilities.ts |
| src/lib/utils/providerConfig.ts | Removed inline ProviderConfigOptions/ValidationResult types, imported from providers.ts/utilities.ts |
| src/lib/types/utilities.ts | New comprehensive utilities types module with 150 lines of extracted types |
| src/lib/types/tools.ts | Converted interfaces to types, added tool execution event types, enhanced with extracted definitions |
| src/lib/types/providers.ts | Added ProviderConfigOptions type, converted namespace interfaces to types |
| src/lib/types/middleware.ts | New middleware-specific types module with factory and configuration types |
| src/lib/types/mcpTypes.ts | Converted interfaces to types, added enhanced MCP types from extracted definitions |
| src/lib/types/index.ts | Added selective exports for new type modules to prevent conflicts |
| src/lib/types/constants.ts | New constants types module with timeout, retry, and performance configuration types |
| src/lib/types/common.ts | Converted interfaces to types, moved tool execution types to tools.ts, added MultimodalInput |
| src/lib/neurolink.ts | Updated imports to use tool execution types from tools.ts instead of common.ts |
| src/lib/middleware/builtin/guardrails.ts | Removed inline GuardrailsMiddlewareConfig, imported from middleware.ts |
| src/lib/core/redisConversationMemoryManager.ts | Removed inline PendingToolExecution interface, imported from tools.ts |
| src/lib/core/conversationMemoryFactory.ts | Removed inline StorageType, imported from common.ts |
| src/lib/core/baseProvider.ts | Removed inline MultimodalInput/ToolCallObject types, imported from common.ts/tools.ts |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
RajuSudhar
force-pushed
the
BZ-44578-types-module-refactor
branch
2 times, most recently
from
October 13, 2025 06:29
cab2362 to
90e679a
Compare
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/lib/types/guardrails.ts (1)
6-118: Preserve interface declarations for guardrails extensibilityThese guardrails contracts used to be interfaces, allowing integrators to augment them with custom issue categories/actions via declaration merging. Converting them to type aliases makes that impossible and introduces a breaking change for existing consumers. Please retain the interface declarations (or introduce an explicit extension mechanism) before merging.
src/lib/types/evaluationTypes.ts (1)
10-150: Restore interfaces for evaluation types
EvaluationResult,EnhancedEvaluationContext, and friends previously allowed augmentation through declaration merging. By switching them totypealiases, we cut off that extension path and force dependent evaluators to refactor, which is a breaking change. Please keep these as interfaces (or supply another sanctioned extension point) to avoid disrupting existing integrations.src/lib/types/conversation.ts (1)
15-304: Maintain interface-based conversation contractsExternal consumers often extend
ChatMessage,SessionMemory, orConversationDatavia declaration merging (e.g., to add tenant-specific metadata). Converting these exports to aliases disallows that and will break downstream builds. Please revert these to interfaces or provide an alternative extension mechanism.src/lib/types/middlewareTypes.ts (1)
10-266: Keep middleware contracts as interfaces for module augmentationMiddleware authors extend
NeuroLinkMiddlewareMetadata,MiddlewareContext, etc., through declaration merging to pass custom config/telemetry. Declaring them astypealiases prevents that and constitutes a breaking change for existing middleware packages. Please stick with interfaces (or expose another extension hook) before proceeding.src/lib/utils/providerConfig.ts (1)
16-24: Fix OpenAI API key regex false negativesOpenAI’s current keys (e.g.,
sk-proj-…,sk-test-…) include hyphenated segments beyond the initialsk-prefix. The new pattern restricts everything aftersk-to[A-Za-z0-9]{48,}, so any hyphenated key now counts as invalid andvalidateApiKeyEnhanced(..., true)will reject legitimate credentials. Please relax the expression to accept the documented formats.- openai: /^sk-[A-Za-z0-9]{48,}$/, + // OpenAI keys now ship with hyphenated segments (sk-proj-…, sk-test-…, sk-live-…, etc.) + openai: /^sk-[A-Za-z0-9_-]{20,}$/,
🧹 Nitpick comments (1)
src/lib/types/sdkTypes.ts (1)
109-119: ExportMultimodalInputvia the SDK aggregator.The new
MultimodalInputtype lives incontent.ts, but the public SDK surface here still omits it. Given this module’s stated goal (“exposes ALL essential types”), consumers can’t reach the new helper without diving into internal paths. Please add it to this export block to keep the public API aligned with the centralized types refactor.export type { TextContent, ImageContent, Content, VisionCapability, ProviderImageFormat, ProcessedImage, MultimodalMessage, + MultimodalInput, ProviderMultimodalPayload, } from "./content.js";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/lib/core/baseProvider.ts(1 hunks)src/lib/core/conversationMemoryFactory.ts(1 hunks)src/lib/core/redisConversationMemoryManager.ts(1 hunks)src/lib/neurolink.ts(1 hunks)src/lib/types/common.ts(4 hunks)src/lib/types/content.ts(1 hunks)src/lib/types/contextTypes.ts(6 hunks)src/lib/types/conversation.ts(11 hunks)src/lib/types/domainTypes.ts(2 hunks)src/lib/types/evaluationTypes.ts(6 hunks)src/lib/types/externalMcp.ts(12 hunks)src/lib/types/guardrails.ts(5 hunks)src/lib/types/middlewareTypes.ts(9 hunks)src/lib/types/providers.ts(2 hunks)src/lib/types/sdkTypes.ts(1 hunks)src/lib/types/streamTypes.ts(3 hunks)src/lib/types/tools.ts(3 hunks)src/lib/types/utilities.ts(1 hunks)src/lib/utils/providerConfig.ts(2 hunks)src/lib/utils/timeout.ts(1 hunks)todos/refactor/07-types-module.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/lib/core/redisConversationMemoryManager.ts
- src/lib/core/baseProvider.ts
- src/lib/types/utilities.ts
- src/lib/utils/timeout.ts
- src/lib/types/streamTypes.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: RajuSudhar
PR: juspay/neurolink#173
File: src/lib/types/index.ts:20-33
Timestamp: 2025-09-17T17:57:36.381Z
Learning: RajuSudhar follows a phased refactor approach to avoid merge conflicts - first consolidating types in focused PRs, then addressing import path updates in dedicated refactor tasks like todos/refactor/07-types-module.md.
📚 Learning: 2025-09-17T17:55:15.261Z
Learnt from: RajuSudhar
PR: juspay/neurolink#173
File: src/lib/index.ts:16-16
Timestamp: 2025-09-17T17:55:15.261Z
Learning: In src/lib/types/providers.ts, ProviderConfig was renamed to AIModelProviderConfig to deduplicate type names, as there was an existing ProviderConfig type that better suited the "ProviderConfig" name. This was an intentional breaking change for better type organization.
Applied to files:
src/lib/types/domainTypes.tssrc/lib/utils/providerConfig.tssrc/lib/types/providers.ts
📚 Learning: 2025-09-28T21:00:08.243Z
Learnt from: RajuSudhar
PR: juspay/neurolink#174
File: src/lib/mcp/contracts/mcpContract.ts:0-0
Timestamp: 2025-09-28T21:00:08.243Z
Learning: The src/lib/mcp/contracts/mcpContract.ts file was completely removed during the MCP types refactor in PR #174, with its types moved to centralized modules like src/lib/types/mcpTypes.ts and src/lib/types/index.ts.
Applied to files:
src/lib/types/externalMcp.ts
📚 Learning: 2025-09-17T17:57:36.381Z
Learnt from: RajuSudhar
PR: juspay/neurolink#173
File: src/lib/types/index.ts:20-33
Timestamp: 2025-09-17T17:57:36.381Z
Learning: RajuSudhar follows a phased refactor approach to avoid merge conflicts - first consolidating types in focused PRs, then addressing import path updates in dedicated refactor tasks like todos/refactor/07-types-module.md.
Applied to files:
src/lib/types/tools.ts
📚 Learning: 2025-09-28T21:09:49.608Z
Learnt from: RajuSudhar
PR: juspay/neurolink#174
File: src/lib/types/tools.ts:166-171
Timestamp: 2025-09-28T21:09:49.608Z
Learning: RajuSudhar prefers to defer type naming collision fixes to follow-up refactor phases when the primary focus is type consolidation and centralization.
Applied to files:
src/lib/types/tools.ts
RajuSudhar
force-pushed
the
BZ-44578-types-module-refactor
branch
from
October 13, 2025 13:47
90e679a to
cf7cc57
Compare
Contributor
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
murdore
approved these changes
Oct 14, 2025
- Move all the type to types/ module
murdore
force-pushed
the
BZ-44578-types-module-refactor
branch
from
October 14, 2025 15:39
cf7cc57 to
95dd9a9
Compare
This was referenced Nov 3, 2025
This was referenced Dec 29, 2025
1 of 69 tasks
2 of 69 tasks
20 tasks done
This was referenced Apr 2, 2026
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
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.
Major reorganization of TypeScript type system from scattered inline definitions to centralized, modular architecture.
Establishes scalable foundation for type system evolution while improving code organization and maintainability across the project.
Pull Request
Description
Type of Change
Related Issues
Changes Made
AI Provider Impact
Component Impact
Testing
Test Environment
Performance Impact
Breaking Changes
Screenshots/Demo
Checklist
Additional Notes
Summary by CodeRabbit
New Features
Refactor
Documentation