feat(notifications): merges notifications swagger spec into api spec #2381
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughLoads and merges an external Notifications OpenAPI spec into generated docs (cached, async), adds scope-aware docs generation and validation on the /v1/doc endpoint, and introduces helpers to filter private routes and subset schemas by references. Changes
Sequence DiagramsequenceDiagram
participant Client
participant REST_App as REST App
participant OpenAPI_Svc as OpenAPI Service
participant Notifications_API as Notifications API
Client->>REST_App: GET /v1/doc?scope=...
REST_App->>REST_App: validate scope (http-assert)
REST_App->>OpenAPI_Svc: generateDocs(handlers, {scope}) (await)
Note over OpenAPI_Svc: Build internal OpenAPI doc (paths, schemas, security)
alt external spec required and not cached
OpenAPI_Svc->>Notifications_API: GET <notifications openapi url>
Notifications_API-->>OpenAPI_Svc: OpenAPI spec (or error)
OpenAPI_Svc->>OpenAPI_Svc: `#filterPrivateRoutes`(paths) → filteredPaths + usedRefs
OpenAPI_Svc->>OpenAPI_Svc: `#filterSchemasByReferences`(schemas, usedRefs)
OpenAPI_Svc->>OpenAPI_Svc: merge filtered external paths, schemas, securitySchemes
OpenAPI_Svc-->>OpenAPI_Svc: cache external spec promise
else cached
OpenAPI_Svc-->>OpenAPI_Svc: use cached external spec
end
OpenAPI_Svc-->>REST_App: combined OpenAPI docs
REST_App-->>Client: JSON response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
ygrishajev
force-pushed
the
feature/notifications-sdk
branch
from
668d3dc to
8eca826
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts(3 hunks)apps/api/src/rest-app.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/rest-app.tsapps/api/src/core/services/openapi-docs/openapi-docs.service.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
🧬 Code graph analysis (1)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (3)
apps/api/src/core/providers/config.provider.ts (2)
CORE_CONFIG(8-8)CoreConfig(14-14)apps/api/src/core/config/env.config.ts (1)
CoreConfig(47-47)apps/api/src/notifications/providers/notifications-config.provider.ts (1)
NOTIFICATIONS_CONFIG(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate (apps/api) / validate-unsafe
- GitHub Check: should-validate / should-validate
- GitHub Check: Decide Whether to Validate / should-validate
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/api/src/rest-app.ts (1)
179-181: LGTM! Async handler correctly awaits external spec loading.The change from synchronous to asynchronous handler is appropriate and necessary to support the new external OpenAPI spec loading in
OpenApiDocsService.generateDocs().apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (2)
2-2: LGTM! Necessary imports for external spec loading.The axios and NotificationsConfig imports support the new functionality to fetch and merge external OpenAPI specs.
Also applies to: 6-7
24-98: LGTM! External spec merging logic is well-structured.The method correctly:
- Awaits external spec loading
- Filters private routes and schemas
- Conditionally merges external paths, schemas, and security schemes
- Handles null external specs gracefully
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
Outdated
Show resolved
Hide resolved
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
Outdated
Show resolved
Hide resolved
ygrishajev
force-pushed
the
feature/notifications-sdk
branch
from
8eca826 to
4cb1776
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| "jest-environment-node": "^29.7.0", | ||
| "jest-junit": "^16.0.0", | ||
| "jest-mock-extended": "^4.0.0-beta1", | ||
| "nock": "^13.5.4", |
There was a problem hiding this comment.
🔄 Carefully review the package-lock.json diff
Resolve the comment if everything is ok
* node_modules/openapi3-ts/node_modules/yaml 2.6.1 -> 2.8.2
* node_modules/openapi3-ts 4.4.0 -> 4.5.0
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (11.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2381 +/- ##
==========================================
- Coverage 51.34% 51.19% -0.16%
==========================================
Files 1071 1071
Lines 29135 29231 +96
Branches 6410 6392 -18
==========================================
+ Hits 14959 14964 +5
- Misses 13782 13846 +64
- Partials 394 421 +27
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (1)
101-123: Configure timeout for axios request to prevent hanging.The
axios.getcall lacks a timeout configuration, which could cause the/v1/docendpoint to hang indefinitely if the external notifications service is slow or unresponsive. This issue was flagged in a previous review and remains unaddressed.🔎 Apply this diff to add timeout:
- const response = await axios.get(externalOpenApiUrl); + const response = await axios.get(externalOpenApiUrl, { + timeout: 5000 // 5 second timeout + });
🧹 Nitpick comments (1)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (1)
25-25: Add explicit return type annotation.The method signature should include an explicit return type for better type safety and documentation.
🔎 Apply this diff:
- async generateDocs(handlers: OpenApiHonoHandler[]) { + async generateDocs(handlers: OpenApiHonoHandler[]): Promise<OpenAPIObject> {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
apps/api/package.json(1 hunks)apps/api/src/core/services/openapi-docs/openapi-docs.service.ts(3 hunks)apps/api/src/rest-app.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/src/rest-app.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use type `any` or cast to type `any`. Always define the proper TypeScript types.
Applied to files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use deprecated methods from libraries.
Applied to files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
🧬 Code graph analysis (1)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (2)
apps/api/src/notifications/providers/notifications-config.provider.ts (1)
NOTIFICATIONS_CONFIG(8-8)apps/api/src/core/services/open-api-hono-handler/open-api-hono-handler.ts (1)
OpenApiHonoHandler(11-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate (apps/api) / should-validate-unsafe / should-validate
- GitHub Check: Decide Whether to Validate / should-validate
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
apps/api/package.json (1)
149-149: LGTM: Appropriate dependency placement.The
openapi3-tspackage is correctly placed indevDependenciessince the service file only uses type imports from it. This avoids bloating the production bundle.apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (6)
2-8: LGTM: Imports are well-structured.The type-only imports from
openapi3-tsand notifications config are properly declared, and axios is correctly imported for the HTTP calls in the external spec loading.
16-16: Good: Proper typing for caching mechanism.The promise cache field is now properly typed with
OpenAPIObjectinstead ofany, addressing previous review feedback. The nullable union type correctly represents the three states: not loaded, loading, and loaded.
18-23: LGTM: Proper dependency injection pattern.The constructor correctly uses TypeScript's DI with the
@injectdecorator and handles the optional notifications configuration gracefully with a null default.
74-96: LGTM: External spec merging logic is well-structured.The conditional merging of external documentation properly handles:
- Filtering private routes before merging
- Collecting and filtering only referenced schemas
- Merging security schemes
- Gracefully handling missing components
129-189: LGTM: Route filtering logic is robust.The method correctly:
- Filters routes by path prefix (
/v1/jobs) and tags (Jobs)- Preserves non-HTTP-method properties like
parametersandservers- Extracts schema references from retained operations for downstream filtering
- Uses proper type guards with
unknowntype
199-247: LGTM: Schema filtering handles transitive dependencies correctly.The method properly:
- Recursively resolves nested schema references (if SchemaA references SchemaB, includes both)
- Prevents infinite loops with the
visitedSchemasSet for circular dependencies- Uses appropriate type safety with
unknownand proper type guards- Handles edge cases like missing schemas gracefully
ygrishajev
force-pushed
the
feature/notifications-sdk
branch
from
4cb1776 to
3b0b3fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (1)
74-96: Consider simplifying the security schemes merge logic.The security schemes merging is duplicated in two branches (lines 86-89 and 91-94). Both branches perform the same operation regardless of whether schemas exist.
🔎 Suggested simplification:
if (externalDocs?.components?.schemas) { const filteredSchemas = this.#filterSchemasByReferences(externalDocs.components.schemas, usedSchemaRefs); docs.components.schemas = { ...docs.components.schemas, ...filteredSchemas }; - docs.components.securitySchemes = { - ...docs.components.securitySchemes, - ...(externalDocs.components.securitySchemes || {}) - }; - } else if (externalDocs?.components) { + } + + if (externalDocs?.components?.securitySchemes) { docs.components.securitySchemes = { ...docs.components.securitySchemes, - ...(externalDocs.components.securitySchemes || {}) + ...externalDocs.components.securitySchemes }; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
apps/api/package.json(1 hunks)apps/api/src/core/services/openapi-docs/openapi-docs.service.ts(3 hunks)apps/api/src/rest-app.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.tsapps/api/src/rest-app.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use type `any` or cast to type `any`. Always define the proper TypeScript types.
Applied to files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use deprecated methods from libraries.
Applied to files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
🧬 Code graph analysis (1)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (1)
apps/api/src/notifications/providers/notifications-config.provider.ts (1)
NOTIFICATIONS_CONFIG(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: should-validate / should-validate
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
apps/api/src/rest-app.ts (1)
179-181: LGTM!The async conversion is correctly implemented. The route now properly awaits the asynchronous
generateDocscall, and any errors will be caught by theHonoErrorHandlerServiceregistered at the app level.apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (4)
1-23: LGTM!The imports use proper OpenAPI types from
openapi3-ts/oas30, and the optional dependency injection pattern forNOTIFICATIONS_CONFIGis well-implemented. The#externalSpecLoadPromisefield is now properly typed withOpenAPIObject.
129-189: LGTM!The filtering logic is well-implemented:
- Correctly handles both path prefix exclusion and tag-based exclusion
- Preserves non-HTTP-method keys (like path-level
parameters)- Uses
Record<string, unknown>instead ofanyfor dynamic assignments- Properly tracks schema references for downstream filtering
199-247: LGTM!The schema filtering logic is robust:
- Uses
visitedSchemasset to prevent infinite loops from circular references- Properly accumulates nested schema references
- The JSDoc accurately describes the behavior
The implementation correctly handles the transitive closure of schema dependencies.
112-120: The axios response is not validated at runtime.While the return type declares
OpenAPIObject, theresponse.datafrom axios is untyped at runtime. If the external API returns malformed data, it could cause unexpected behavior when processing in#filterPrivateRoutes.Consider adding minimal runtime validation or explicit type assertion with a safety comment.
Does openapi3-ts provide validation utilities for OpenAPIObject?
ygrishajev
force-pushed
the
feature/notifications-sdk
branch
from
1f91193 to
612fd41
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/api/test/functional/docs.spec.ts (1)
9-9: Consider adding test coverage for different scope values.The test now correctly exercises the
scope=consoleparameter. Consider adding additional test cases to cover:
- The
scope=fullparameter (which triggers external spec loading)- Invalid scope values (error handling)
- Missing scope parameter (default behavior)
This would provide more comprehensive coverage of the new scope-aware documentation feature.
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (2)
25-25: Prefer stricter typing for the scope parameter.The
scopeparameter is typed asstring, which allows any value without compile-time validation. Consider using a union type or enum for better type safety.🔎 Apply this diff to add stricter typing:
Define the scope type near the top of the file or in a types file:
type DocsScope = "console" | "full";Then update the method signature:
-async generateDocs(handlers: OpenApiHonoHandler[], options: { scope: string }) { +async generateDocs(handlers: OpenApiHonoHandler[], options: { scope: DocsScope }) {This provides compile-time validation and better IDE autocomplete support.
103-125: Consider cache invalidation strategy for long-running processes.The external spec is cached for the lifetime of the process via
#externalSpecLoadPromise. While this is efficient, it means the external spec will never be refreshed until the service restarts.For long-running processes, consider implementing:
- A TTL-based cache invalidation strategy
- A manual cache refresh endpoint (if needed)
For typical deployment patterns with regular restarts, the current implementation is acceptable.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts(3 hunks)apps/api/src/rest-app.ts(2 hunks)apps/api/test/functional/docs.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/src/rest-app.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.tsapps/api/test/functional/docs.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/api/test/functional/docs.spec.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use type `any` or cast to type `any`. Always define the proper TypeScript types.
Applied to files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use deprecated methods from libraries.
Applied to files:
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/api/test/functional/docs.spec.ts
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/api/test/functional/docs.spec.ts
🧬 Code graph analysis (1)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (3)
apps/api/src/core/providers/config.provider.ts (2)
CORE_CONFIG(8-8)CoreConfig(14-14)apps/api/src/core/config/env.config.ts (1)
CoreConfig(47-47)apps/api/src/notifications/providers/notifications-config.provider.ts (1)
NOTIFICATIONS_CONFIG(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: validate (apps/provider-console) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (5)
apps/api/src/core/services/openapi-docs/openapi-docs.service.ts (5)
127-191: LGTM! Private route filtering logic is well-implemented.The method correctly:
- Filters routes by path prefix and operation tags
- Preserves non-HTTP-method keys (like
parameters,servers)- Tracks schema references for downstream filtering
- Handles edge cases appropriately
The excluded tags and path prefixes are currently hardcoded. If these need to be configurable in the future, consider creating a separate issue to track that enhancement.
Based on learnings, cross-cutting refactoring concerns should be tracked separately rather than expanding the current PR scope.
193-249: LGTM! Schema filtering logic is robust.The method correctly:
- Recursively resolves nested schema references
- Prevents infinite loops with
visitedSchemastracking- Handles circular dependencies properly
- Returns only the schemas actually referenced by included routes
The implementation is thorough and handles all edge cases appropriately.
18-23: LGTM! Dependency injection setup is correct.The constructor properly injects
NotificationsConfigusing the DI token, and the optional default value allows the service to work without notifications configuration.
2-3: LGTM! Type safety has been significantly improved.The imports and type definitions properly use
OpenAPIObject,PathsObject,ComponentsObject, andReferenceObjectfromopenapi3-ts/oas30, addressing the previous review concerns aboutanytypes.The
#externalSpecLoadPromisefield is now correctly typed asPromise<OpenAPIObject | null> | null.As per coding guidelines,
anytypes have been eliminated in favor of proper OpenAPI types.Also applies to: 7-8, 16-16
74-98: Confirm the path merge precedence is intentional and document or test it.The code merges internal handler paths first, then external notifications paths via
Object.assign(docs.paths, filteredPaths). This means external paths will overwrite any internal paths sharing the same key. No tests or documentation currently clarify whether this precedence is intentional. Either add a test confirming this behavior is desired, or update the merge strategy if paths should be kept separate or conflicts should be detected.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.