-
Notifications
You must be signed in to change notification settings - Fork 4
Add support for listing and deleting streams #200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds stream listing and deletion APIs with types and tests; bumps package version and changelog. Tightens several public types, replaces core imports with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as StreamAPIImpl
participant HTTP as HTTP Client
rect rgba(221,235,247,0.6)
note right of API: List Streams (new)
end
Client->>API: list(params?)
API->>API: validate params (limit, offset, name, metadata)
API->>HTTP: POST /streams/list {filters, pagination}
HTTP-->>API: 200 {items,total,limit,offset} / 400 {error} / other
alt 200 OK
API-->>Client: ListStreamsResponse
else 400 Bad Request
API-->>Client: throw ValidationError
else Other
API-->>Client: throw Error(status, body)
end
sequenceDiagram
autonumber
actor Client
participant API as StreamAPIImpl
participant HTTP as HTTP Client
rect rgba(247,231,215,0.6)
note right of API: Delete Stream (new)
end
Client->>API: delete(id)
API->>API: validate id (non-empty string)
API->>HTTP: DELETE /streams/{id}
HTTP-->>API: 200/204 / 404 / other
alt 200/204
API-->>Client: void
else 404 Not Found
API-->>Client: throw NotFoundError
else Other
API-->>Client: throw Error(status, body)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/types.ts (1)
576-586: Document breaking change for VectorSearchParams
The generic constraint was tightened fromT = unknowntoT extends JsonObject, which may break consumers passing non-JSON-object metadata—add a corresponding entry to CHANGELOG.md.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)package.json(1 hunks)src/apis/stream.ts(2 hunks)src/types.ts(2 hunks)test/stream.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/apis/**
📄 CodeRabbit inference engine (AGENT.md)
Place core API implementations under src/apis/ (email, discord, keyvalue, vector, objectstore)
Files:
src/apis/stream.ts
{src,test}/**/!(*.d).ts
📄 CodeRabbit inference engine (AGENT.md)
{src,test}/**/!(*.d).ts: Use strict TypeScript and prefer unknown over any
Use ESM import/export syntax; avoid CommonJS require/module.exports
Use relative imports for internal modules
Keep imports organized (sorted, no unused imports)
Use tabs with a visual width of 2 spaces
Limit lines to a maximum of 80 characters
Use single quotes for strings
Use proper Error types; do not throw strings
Prefer template literals over string concatenation
Files:
src/apis/stream.tssrc/types.tstest/stream.test.ts
src/apis/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/code-generation.mdc)
src/apis/**/*.ts: Do not hardcode generated prompt content (e.g., copyWriter) in source files; load it dynamically
Avoid overly complex TypeScript generics for generated content; prefer simple, maintainable types
Maintain type safety for dynamically loaded content by generating and referencing TypeScript definitions, with proper annotations for require() results
Do not use relative imports to generated artifacts; resolve via absolute node_modules paths or the package entry
Generated content is loaded at runtime, not build time; avoid static imports of generated modules
Prefer bracket notation for accessing slug-named properties with hyphens (e.g., prompts['slug-name'])
Avoid relative require('./generated/_index.js'); resolve absolute paths from process.cwd() into node_modules for generated assets
Files:
src/apis/stream.ts
test/**
📄 CodeRabbit inference engine (AGENT.md)
Tests must mirror the source structure under the test/ directory
Files:
test/stream.test.ts
🧬 Code graph analysis (2)
src/apis/stream.ts (3)
src/types.ts (2)
ListStreamsParams(359-379)ListStreamsResponse(414-434)src/router/router.ts (2)
getTracer(61-66)recordException(108-128)src/apis/api.ts (2)
POST(244-261)DELETE(302-319)
test/stream.test.ts (1)
src/apis/api.ts (1)
setFetch(9-11)
🪛 Biome (2.1.2)
test/stream.test.ts
[error] 1681-1682: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 1723-1724: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Actions: Build
test/stream.test.ts
[warning] 1681-1683: lint/suspicious/noDuplicateObjectKeys: This property is later overwritten by an object member with the same name.
[warning] 1721-1725: lint/suspicious/noDuplicateObjectKeys: This property is later overwritten by an object member with the same name.
package.json
[warning] vulnerability: npm audit reveals issues. Run 'npm audit fix'.
🔇 Additional comments (9)
package.json (1)
3-3: Version bump looks correct.The version bump to 0.0.156 aligns with the new stream listing and deletion features introduced in this PR.
CHANGELOG.md (1)
3-137: Well-documented changelog entry.The changelog entry comprehensively documents the new stream listing and deletion features with clear examples, parameter descriptions, and error handling notes. This will help users understand how to use the new functionality.
test/stream.test.ts (2)
1744-1829: Comprehensive test coverage for list method.The test suite thoroughly covers various listing scenarios including filters, pagination, validation, and combined parameters. This provides good confidence in the list functionality.
1881-1915: Good validation coverage for delete method.The tests properly validate input parameters including empty strings, non-string types, and whitespace-only IDs. This ensures robust error handling for the delete operation.
src/apis/stream.ts (3)
436-518: Well-structured list implementation.The list method follows the established patterns in the codebase with proper tracing, error handling, and context management. The implementation correctly handles various response status codes.
526-574: Robust delete implementation.The delete method includes proper input validation, tracing, and handles multiple success status codes (200 and 204), which is good practice for DELETE operations. The 404 handling provides clear error messaging.
447-450: Fix limit validation to match documentation.The validation rejects
limit > 1000, but the documentation in ListStreamsParams (line 371 in src/types.ts) states "max: 1000", which typically means 1000 should be allowed. The condition should be>= 1001or> 1000needs to be changed to allow 1000.Apply this diff to allow limit=1000:
if (params?.limit !== undefined) { - if (params.limit <= 0 || params.limit > 1000) { + if (params.limit <= 0 || params.limit >= 1001) { throw new Error('limit must be greater than 0 and less than 1000'); } span.setAttribute('limit', params.limit); }Alternatively, update the error message to clarify the maximum:
if (params?.limit !== undefined) { if (params.limit <= 0 || params.limit > 1000) { - throw new Error('limit must be greater than 0 and less than 1000'); + throw new Error('limit must be between 1 and 1000 (inclusive)'); } span.setAttribute('limit', params.limit); }Likely an incorrect or invalid review comment.
src/types.ts (2)
356-434: Well-defined types for stream listing.The new interfaces (ListStreamsParams, StreamInfo, ListStreamsResponse) are clearly documented with JSDoc comments and provide appropriate structure for the list operation. The types support filtering, pagination, and comprehensive response information.
496-511: StreamAPI interface properly extended.The list and delete methods are correctly added to the StreamAPI interface with clear documentation and appropriate signatures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
test/utils/interpolate.test.ts (1)
66-69: Prefer template literals over concatenation for clarityUse escaped template literals to avoid concatenation.
As per coding guidelines
- input: 'this is a $' + '{test}', + input: `this is a \${test}`,- input: 'this is a $' + '{test:-foo}', + input: `this is a \${test:-foo}`,Also applies to: 71-74
test/stream.test.ts (1)
1830-1910: Add 204 No Content delete caseSpec allows 200/204; add a test to assert 204 is handled.
beforeEach(() => { const mockFetch = mock( async (url: URL | RequestInfo, options?: RequestInit) => { if (options?.method === 'DELETE') { const urlStr = url.toString(); // Check for valid stream ID if (urlStr.includes('/stream-123')) { - return { - status: 200, + return { + status: 200, response: { status: 200, statusText: 'OK', headers: new Headers(), }, headers: new Headers(), }; } + if (urlStr.includes('/stream-204')) { + return { + status: 204, + response: { + status: 204, + statusText: 'No Content', + headers: new Headers(), + }, + headers: new Headers(), + }; + }it('should delete a stream successfully', async () => { await expect(streamAPI.delete('stream-123')).resolves.toBeUndefined(); }); + + it('should handle 204 No Content on delete', async () => { + await expect(streamAPI.delete('stream-204')).resolves.toBeUndefined(); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
package.json(2 hunks)src/apis/patchportal.ts(2 hunks)src/apis/prompt/generic_types.ts(1 hunks)src/apis/prompt/index.ts(2 hunks)src/apis/stream.ts(2 hunks)src/autostart/index.ts(4 hunks)src/index.ts(1 hunks)src/otel/index.ts(4 hunks)src/otel/logger.ts(1 hunks)src/utils/promptMetadata.ts(2 hunks)test/stream.test.ts(1 hunks)test/utils/interpolate.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/apis/prompt/index.ts
- src/otel/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/apis/stream.ts
- package.json
🧰 Additional context used
📓 Path-based instructions (8)
src/apis/**
📄 CodeRabbit inference engine (AGENT.md)
Place core API implementations under src/apis/ (email, discord, keyvalue, vector, objectstore)
Files:
src/apis/patchportal.tssrc/apis/prompt/generic_types.ts
{src,test}/**/!(*.d).ts
📄 CodeRabbit inference engine (AGENT.md)
{src,test}/**/!(*.d).ts: Use strict TypeScript and prefer unknown over any
Use ESM import/export syntax; avoid CommonJS require/module.exports
Use relative imports for internal modules
Keep imports organized (sorted, no unused imports)
Use tabs with a visual width of 2 spaces
Limit lines to a maximum of 80 characters
Use single quotes for strings
Use proper Error types; do not throw strings
Prefer template literals over string concatenation
Files:
src/apis/patchportal.tssrc/index.tssrc/apis/prompt/generic_types.tstest/stream.test.tssrc/utils/promptMetadata.tssrc/otel/logger.tssrc/autostart/index.tstest/utils/interpolate.test.ts
src/apis/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/code-generation.mdc)
src/apis/**/*.ts: Do not hardcode generated prompt content (e.g., copyWriter) in source files; load it dynamically
Avoid overly complex TypeScript generics for generated content; prefer simple, maintainable types
Maintain type safety for dynamically loaded content by generating and referencing TypeScript definitions, with proper annotations for require() results
Do not use relative imports to generated artifacts; resolve via absolute node_modules paths or the package entry
Generated content is loaded at runtime, not build time; avoid static imports of generated modules
Prefer bracket notation for accessing slug-named properties with hyphens (e.g., prompts['slug-name'])
Avoid relative require('./generated/_index.js'); resolve absolute paths from process.cwd() into node_modules for generated assets
Files:
src/apis/patchportal.tssrc/apis/prompt/generic_types.ts
src/index.ts
📄 CodeRabbit inference engine (AGENT.md)
src/index.ts must be the entry point and export server, logger, types, and APIs
Always export interpolateTemplate from the main SDK index
Files:
src/index.ts
src/apis/prompt/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/code-generation.mdc)
src/apis/prompt/**/*.ts: In TS interfaces that include hyphenated slugs, quote property names (e.g., 'slug-name') and access via bracket notation
Generated content must import from @agentuity/sdk rather than relative paths
Files:
src/apis/prompt/generic_types.ts
src/apis/prompt/generic_types.ts
📄 CodeRabbit inference engine (.cursor/rules/code-generation.mdc)
Keep types simple: define PromptsCollection as Record<string, any> and PromptSignature as (params: any) => string; avoid complex generics
Files:
src/apis/prompt/generic_types.ts
test/**
📄 CodeRabbit inference engine (AGENT.md)
Tests must mirror the source structure under the test/ directory
Files:
test/stream.test.tstest/utils/interpolate.test.ts
src/{logger,otel}/**
📄 CodeRabbit inference engine (AGENT.md)
Infrastructure code goes under src/logger/ and src/otel/ (OpenTelemetry)
Files:
src/otel/logger.ts
🧠 Learnings (2)
📚 Learning: 2025-10-03T22:20:06.957Z
Learnt from: CR
PR: agentuity/sdk-js#0
File: .cursor/rules/code-generation.mdc:0-0
Timestamp: 2025-10-03T22:20:06.957Z
Learning: Applies to src/apis/prompt/generic_types.ts : Keep types simple: define PromptsCollection as Record<string, any> and PromptSignature<T> as (params: any) => string; avoid complex generics
Applied to files:
src/apis/prompt/generic_types.ts
📚 Learning: 2025-08-25T13:53:20.040Z
Learnt from: CR
PR: agentuity/sdk-js#0
File: AGENT.md:0-0
Timestamp: 2025-08-25T13:53:20.040Z
Learning: Applies to {src,test}/**/!(*.d).ts : Prefer template literals over string concatenation
Applied to files:
test/utils/interpolate.test.ts
🧬 Code graph analysis (1)
test/stream.test.ts (1)
src/apis/api.ts (1)
setFetch(9-11)
🔇 Additional comments (8)
src/autostart/index.ts (4)
4-8: LGTM: Import formatting.The import organization follows the coding guidelines for keeping imports organized and readable.
14-15: LGTM: Type-only imports improve tree-shaking.Converting to type-only imports is a best practice since these types are only used in type positions (line 133-136). This enables better tree-shaking and reduces bundle size.
132-138: LGTM: Explicit type annotation improves clarity.The expanded type annotation makes the structure of
userLoggerProviderexplicit and aligns with its usage throughout the function (lines 145, 151, 182-192). This improves type safety and code readability.
96-96: LGTM: Formatting improvements.These formatting changes improve readability and comply with the 80-character line limit guideline.
Also applies to: 153-155
src/otel/logger.ts (1)
61-68: LGTM! Formatting improvements align with coding guidelines.The changes improve adherence to the 80-character line limit guideline by splitting the method signature across multiple lines. The addition of parentheses around the arrow function parameter maintains consistency with other arrow functions in the file (lines 90, 101, 112, 123, and throughout the
patchConsolefunction).src/utils/promptMetadata.ts (2)
1-1: LGTM! Good use ofnode:prefix for built-in module.The
node:prefix explicitly identifies this as a Node.js built-in module, following modern Node.js best practices and preventing potential conflicts with npm packages.
26-27: LGTM! Template literals improve readability.The change from string concatenation to template literals aligns with the coding guidelines and improves code readability.
test/stream.test.ts (1)
1658-1828: List tests look comprehensiveGood coverage for filters, pagination, shape, and limits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/index.ts (2)
54-63: Good fix, but wrap the warning message for line length.The try/catch implementation correctly addresses the past review comment and prevents the runner from crashing on invalid JSON. However, line 60 exceeds the 80-character line limit specified in the coding guidelines.
Apply this diff to wrap the warning message:
try { userOtelConf = JSON.parse(process.env.AGENTUITY_USER_OTEL_CONF); } catch (error) { console.warn( - `[WARN] Failed to parse AGENTUITY_USER_OTEL_CONF: ${error instanceof Error ? error.message : String(error)}` + '[WARN] Failed to parse AGENTUITY_USER_OTEL_CONF:', + error instanceof Error ? error.message : String(error) ); }As per coding guidelines
48-48: Apply the same JSON parsing protection to AGENTUITY_CLOUD_AGENTS_JSON.Line 48 also uses
JSON.parseon environment data without error handling, creating the same crash risk that was just fixed forAGENTUITY_USER_OTEL_CONF.Apply this diff to add try/catch protection:
let agents: AgentConfig[] = []; if (agentsJSON) { - agents = JSON.parse(agentsJSON); + try { + agents = JSON.parse(agentsJSON); + } catch (error) { + console.warn( + '[WARN] Failed to parse AGENTUITY_CLOUD_AGENTS_JSON:', + error instanceof Error ? error.message : String(error) + ); + } } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/apis/patchportal.ts(2 hunks)src/autostart/index.ts(4 hunks)src/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/autostart/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
src/index.ts
📄 CodeRabbit inference engine (AGENT.md)
src/index.ts must be the entry point and export server, logger, types, and APIs
Always export interpolateTemplate from the main SDK index
Files:
src/index.ts
{src,test}/**/!(*.d).ts
📄 CodeRabbit inference engine (AGENT.md)
{src,test}/**/!(*.d).ts: Use strict TypeScript and prefer unknown over any
Use ESM import/export syntax; avoid CommonJS require/module.exports
Use relative imports for internal modules
Keep imports organized (sorted, no unused imports)
Use tabs with a visual width of 2 spaces
Limit lines to a maximum of 80 characters
Use single quotes for strings
Use proper Error types; do not throw strings
Prefer template literals over string concatenation
Files:
src/index.tssrc/apis/patchportal.ts
src/apis/**
📄 CodeRabbit inference engine (AGENT.md)
Place core API implementations under src/apis/ (email, discord, keyvalue, vector, objectstore)
Files:
src/apis/patchportal.ts
src/apis/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/code-generation.mdc)
src/apis/**/*.ts: Do not hardcode generated prompt content (e.g., copyWriter) in source files; load it dynamically
Avoid overly complex TypeScript generics for generated content; prefer simple, maintainable types
Maintain type safety for dynamically loaded content by generating and referencing TypeScript definitions, with proper annotations for require() results
Do not use relative imports to generated artifacts; resolve via absolute node_modules paths or the package entry
Generated content is loaded at runtime, not build time; avoid static imports of generated modules
Prefer bracket notation for accessing slug-named properties with hyphens (e.g., prompts['slug-name'])
Avoid relative require('./generated/_index.js'); resolve absolute paths from process.cwd() into node_modules for generated assets
Files:
src/apis/patchportal.ts
🧬 Code graph analysis (1)
src/index.ts (1)
src/autostart/index.ts (1)
UserOpenTelemetryConfig(21-28)
🔇 Additional comments (2)
src/apis/patchportal.ts (2)
68-70: LGTM! Critical null handling issue resolved.The guard
data && typeof data === 'object'correctly prevents the crash whendatais null (sincetypeof null === 'object'in JavaScript). This addresses the past review comment by ensuringObject.keys()is never called on null.Note: The
data &&check will also short-circuit on other falsy values (0,'',false, etc.), which is reasonable for debug logging since these wouldn't produce usefulObject.keys()output anyway.
84-84: LGTM! Type assertion consistent with set method.The
as Record<string, unknown>type assertion matches the pattern used in thesetmethod (line 69). The null safety was already present via theresult ?check on line 82-83, so this is purely a type-safety improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apis/stream.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/apis/**
📄 CodeRabbit inference engine (AGENT.md)
Place core API implementations under src/apis/ (email, discord, keyvalue, vector, objectstore)
Files:
src/apis/stream.ts
{src,test}/**/!(*.d).ts
📄 CodeRabbit inference engine (AGENT.md)
{src,test}/**/!(*.d).ts: Use strict TypeScript and prefer unknown over any
Use ESM import/export syntax; avoid CommonJS require/module.exports
Use relative imports for internal modules
Keep imports organized (sorted, no unused imports)
Use tabs with a visual width of 2 spaces
Limit lines to a maximum of 80 characters
Use single quotes for strings
Use proper Error types; do not throw strings
Prefer template literals over string concatenation
Files:
src/apis/stream.ts
src/apis/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/code-generation.mdc)
src/apis/**/*.ts: Do not hardcode generated prompt content (e.g., copyWriter) in source files; load it dynamically
Avoid overly complex TypeScript generics for generated content; prefer simple, maintainable types
Maintain type safety for dynamically loaded content by generating and referencing TypeScript definitions, with proper annotations for require() results
Do not use relative imports to generated artifacts; resolve via absolute node_modules paths or the package entry
Generated content is loaded at runtime, not build time; avoid static imports of generated modules
Prefer bracket notation for accessing slug-named properties with hyphens (e.g., prompts['slug-name'])
Avoid relative require('./generated/_index.js'); resolve absolute paths from process.cwd() into node_modules for generated assets
Files:
src/apis/stream.ts
🧬 Code graph analysis (1)
src/apis/stream.ts (3)
src/types.ts (2)
ListStreamsParams(359-379)ListStreamsResponse(414-434)src/router/router.ts (2)
getTracer(61-66)recordException(108-128)src/apis/api.ts (2)
POST(244-261)DELETE(302-319)
🔇 Additional comments (1)
src/apis/stream.ts (1)
522-571: LGTM!The delete method implementation is well-structured with appropriate input validation, error handling, and status code checks. The pattern is consistent with other methods in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
test/stream.test.ts (2)
1673-1683: Remove duplicatejsonproperty.The mock response object defines the
jsonproperty twice (line 1682 and again on line 1682 in the return). The first assignment is overwritten by the second, making it redundant.Apply this diff to remove the duplicate:
return { status: 400, response: { status: 400, statusText: 'Bad Request', headers: new Headers({ 'content-type': 'application/json' }), json: () => Promise.resolve(errorJson), }, - json: errorJson, headers: new Headers({ 'content-type': 'application/json' }), json: () => Promise.resolve(errorJson), };Note: This issue was flagged in a previous review but appears to remain unresolved.
1714-1724: Remove duplicatejsonproperty.The mock response object defines the
jsonproperty twice (line 1723 first as a plain value, then as a function). The first assignment is overwritten by the second, making it redundant.Apply this diff to remove the duplicate:
return { status: 200, response: { status: 200, statusText: 'OK', headers: new Headers({ 'content-type': 'application/json' }), json: () => Promise.resolve(successJson), }, - json: successJson, headers: new Headers({ 'content-type': 'application/json' }), json: () => Promise.resolve(successJson), };Note: This issue was flagged in a previous review but appears to remain unresolved.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/apis/stream.ts(2 hunks)test/stream.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apis/stream.ts
🧰 Additional context used
📓 Path-based instructions (2)
test/**
📄 CodeRabbit inference engine (AGENT.md)
Tests must mirror the source structure under the test/ directory
Files:
test/stream.test.ts
{src,test}/**/!(*.d).ts
📄 CodeRabbit inference engine (AGENT.md)
{src,test}/**/!(*.d).ts: Use strict TypeScript and prefer unknown over any
Use ESM import/export syntax; avoid CommonJS require/module.exports
Use relative imports for internal modules
Keep imports organized (sorted, no unused imports)
Use tabs with a visual width of 2 spaces
Limit lines to a maximum of 80 characters
Use single quotes for strings
Use proper Error types; do not throw strings
Prefer template literals over string concatenation
Files:
test/stream.test.ts
🧬 Code graph analysis (1)
test/stream.test.ts (1)
src/apis/api.ts (1)
setFetch(9-11)
🔇 Additional comments (2)
test/stream.test.ts (2)
1742-1827: LGTM! Comprehensive test coverage for list endpoint.The test suite thoroughly covers the list endpoint functionality:
- No filters
- Name filter
- Metadata filter
- Pagination (limit, offset)
- Limit validation (boundaries and invalid ranges)
- Response structure validation
- Combined filters
The tests properly verify the API contract and edge cases.
1830-1910: LGTM! Thorough delete endpoint validation.The test suite comprehensively validates the delete endpoint:
- Successful deletion
- Not-found error handling
- Input validation (empty, null, undefined, whitespace-only)
The tests correctly use type casting (
as unknown as string) to verify runtime type checking, and all error messages are consistent with expectations.
| if (body.limit && (body.limit <= 0 || body.limit > 1000)) { | ||
| const errorJson = { | ||
| success: false, | ||
| message: 'limit must be greater than 0 and less than 1000', | ||
| streams: [], | ||
| total: 0, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix inconsistent limit validation logic and error message.
The mock validation logic and error message are inconsistent with the test expectations:
- Line 1666: Checks
body.limit <= 0 || body.limit > 1000(excludes 1000) - Line 1669: Returns error "limit must be greater than 0 and less than 1000"
- Line 1788, 1792: Tests expect error "limit must be greater than 0 and less than or equal to 1000" (includes 1000)
This mismatch means the test at line 1800 (limit: 1000) would incorrectly pass due to the mock allowing 1000, even though the error message suggests otherwise.
Apply this diff to align the mock with the test expectations:
- if (body.limit && (body.limit <= 0 || body.limit > 1000)) {
+ if (body.limit && (body.limit <= 0 || body.limit > 1000)) {
const errorJson = {
success: false,
- message: 'limit must be greater than 0 and less than 1000',
+ message: 'limit must be greater than 0 and less than or equal to 1000',
streams: [],
total: 0,
};📝 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.
| if (body.limit && (body.limit <= 0 || body.limit > 1000)) { | |
| const errorJson = { | |
| success: false, | |
| message: 'limit must be greater than 0 and less than 1000', | |
| streams: [], | |
| total: 0, | |
| }; | |
| if (body.limit && (body.limit <= 0 || body.limit > 1000)) { | |
| const errorJson = { | |
| success: false, | |
| message: 'limit must be greater than 0 and less than or equal to 1000', | |
| streams: [], | |
| total: 0, | |
| }; |
🤖 Prompt for AI Agents
In test/stream.test.ts around lines 1666 to 1672 the validation condition allows
a limit of 1000 but the error message says "less than 1000", which contradicts
the tests that expect "less than or equal to 1000"; update the error message
string in the mocked response to "limit must be greater than 0 and less than or
equal to 1000" so it matches the validation logic and the test expectations.
Stream API Updates
New Features
List Streams
You can now list and search through your streams with flexible filtering and pagination options.
Response Format:
success: Boolean indicating if the request succeededstreams: Array of stream objects withid,name,metadata,url, andsizeBytestotal: Total count of streams matching your filters (useful for pagination)message: Optional error message if request failedPagination:
offsetto skip streams for paginationDelete Streams
You can now delete streams by their ID when they're no longer needed.
Notes:
Complete Stream Workflow Example
Migration Notes
If you're currently managing streams, you can now:
API Reference
streams.list(params?)Parameters:
name(optional): Filter streams by namemetadata(optional): Object with key-value pairs to filter by metadatalimit(optional): Maximum streams to return (1-1000, default: 100)offset(optional): Number of streams to skip for paginationReturns:
Promise<ListStreamsResponse>streams.delete(id)Parameters:
id(required): The stream ID to deleteReturns:
Promise<void>Throws:
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests
Chores