[ENGG-5253]: Add support for examples in local workspaces#300
Open
RuntimeTerror10 wants to merge 3 commits intomasterfrom
Open
[ENGG-5253]: Add support for examples in local workspaces#300RuntimeTerror10 wants to merge 3 commits intomasterfrom
RuntimeTerror10 wants to merge 3 commits intomasterfrom
Conversation
WalkthroughAdds example-request support across local-sync: new types ResponseObject, ExampleRecord, and ExampleAPI; a parser parseExampleContentIntoRecord; FsManager methods createExampleRequest, updateExampleRequest, and deleteExampleRequest; refactors getAllRecords to branch folder vs file processing and include parsed examples for files; and exposes the three new FsManager methods over IPC in FsManagerRPCService. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/actions/local-sync/fs-manager.ts`:
- Around line 1621-1673: The not-found branch in deleteExampleRequest currently
returns ErrorCode.UNKNOWN; change it to return ErrorCode.NotFound (or the
project's canonical NotFound enum value) so callers can distinguish a missing
example from other errors—update the error object in deleteExampleRequest
(referencing requestFileResource, content.examples check and the returned error)
to use ErrorCode.NotFound and make the same change in the analogous not-found
branches around the other example-handling function referenced (the block at the
1726-1784 region).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/renderer/actions/local-sync/fs-manager.rpc-service.tssrc/renderer/actions/local-sync/fs-manager.tssrc/renderer/actions/local-sync/fs-utils.tssrc/renderer/actions/local-sync/schemas.tssrc/renderer/actions/local-sync/types.ts
Aarushsr12
requested changes
Feb 27, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/actions/local-sync/fs-manager.ts (1)
747-761: Consider logging or tracking example parsing failures.Currently, if
parseExampleContentIntoRecordreturns an error, it's silently ignored. While examples may be non-critical, tracking these failures would help with debugging malformed data.♻️ Proposed enhancement to track example errors
const { content: record } = fileResult; if (record.examples) { const parentRequestId = getIdFromPath(resource.path); for (const [exampleId, exampleContent] of Object.entries( record.examples )) { const exampleResult = parseExampleContentIntoRecord( exampleContent, exampleId, parentRequestId ); if (exampleResult.type === "success") { entities.push(exampleResult.content); + } else { + console.warn( + `Failed to parse example ${exampleId} in ${resource.path}:`, + exampleResult.error?.message + ); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/actions/local-sync/fs-manager.ts` around lines 747 - 761, When iterating record.examples in the block that calls parseExampleContentIntoRecord, add handling for non-success returns so parsing failures are not silent: detect when exampleResult.type !== "success" and record or log the failure (include exampleId, parentRequestId from getIdFromPath(resource.path), and the exampleResult error/message). Update the loop around parseExampleContentIntoRecord to push successful contents into entities as before and to call the existing logger/tracker (or a new error-collection structure) for failed parses so malformed example data is discoverable during debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/actions/local-sync/fs-manager.ts`:
- Around line 747-761: When iterating record.examples in the block that calls
parseExampleContentIntoRecord, add handling for non-success returns so parsing
failures are not silent: detect when exampleResult.type !== "success" and record
or log the failure (include exampleId, parentRequestId from
getIdFromPath(resource.path), and the exampleResult error/message). Update the
loop around parseExampleContentIntoRecord to push successful contents into
entities as before and to call the existing logger/tracker (or a new
error-collection structure) for failed parses so malformed example data is
discoverable during debugging.
Aarushsr12
approved these changes
Feb 27, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/actions/local-sync/schemas.ts (1)
65-73: Reduce schema drift betweenResponseHeaderandKeyValuePair.
ResponseHeaderandKeyValuePaircurrently duplicate near-identical fields. This will drift over time. Consider extracting a shared header schema and deriving required/optional variants from it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/actions/local-sync/schemas.ts` around lines 65 - 73, Extract the common fields from ResponseHeader and KeyValuePair into a shared schema (e.g., BaseKeyValue or HeaderBase) that includes key, value, dataType (Type.Enum(KeyValueDataType)), and any common optional metadata like description and type; then redefine ResponseHeader and KeyValuePair by composing/augmenting that shared schema and applying required/optional wrappers (e.g., Type.Omit/Type.Partial or separate Required/Optional variants) so both are derived from the same source and avoid duplication while preserving existing optional fields like id and isEnabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/actions/local-sync/schemas.ts`:
- Around line 148-156: Tighten the ResponseObject schema by replacing the loose
numeric types: change the status property in ResponseObject from Type.Number()
to Type.Integer({ minimum: 100, maximum: 599 }) to enforce valid HTTP status
codes, and change time from Type.Optional(Type.Number()) to
Type.Optional(Type.Integer({ minimum: 0 })) so time cannot be negative; keep the
same property names and optional/required semantics for ResponseObject,
ResponseObject.status, and ResponseObject.time when making the change.
---
Nitpick comments:
In `@src/renderer/actions/local-sync/schemas.ts`:
- Around line 65-73: Extract the common fields from ResponseHeader and
KeyValuePair into a shared schema (e.g., BaseKeyValue or HeaderBase) that
includes key, value, dataType (Type.Enum(KeyValueDataType)), and any common
optional metadata like description and type; then redefine ResponseHeader and
KeyValuePair by composing/augmenting that shared schema and applying
required/optional wrappers (e.g., Type.Omit/Type.Partial or separate
Required/Optional variants) so both are derived from the same source and avoid
duplication while preserving existing optional fields like id and isEnabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3661482a-7d38-4d05-a5da-24d5c9e61f7a
📒 Files selected for processing (1)
src/renderer/actions/local-sync/schemas.ts
Comment on lines
+148
to
+156
| export const ResponseObject = Type.Object({ | ||
| body: Type.String(), | ||
| headers: Type.Array(ResponseHeader), | ||
| method: Type.Optional(Type.Enum(ApiMethods)), | ||
| status: Type.Number(), | ||
| statusText: Type.Optional(Type.String()), | ||
| time: Type.Optional(Type.Number()), | ||
| redirectedUrl: Type.Optional(Type.String()), | ||
| }); |
There was a problem hiding this comment.
Tighten HTTP response numeric validation.
At Line 152, status: Type.Number() allows invalid HTTP statuses (e.g., fractional/negative). This should be constrained to valid integer status codes; similarly, time should not be negative.
Proposed schema tightening
export const ResponseObject = Type.Object({
body: Type.String(),
headers: Type.Array(ResponseHeader),
method: Type.Optional(Type.Enum(ApiMethods)),
- status: Type.Number(),
+ status: Type.Integer({ minimum: 100, maximum: 599 }),
statusText: Type.Optional(Type.String()),
- time: Type.Optional(Type.Number()),
+ time: Type.Optional(Type.Number({ minimum: 0 })),
redirectedUrl: Type.Optional(Type.String()),
});📝 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.
Suggested change
| export const ResponseObject = Type.Object({ | |
| body: Type.String(), | |
| headers: Type.Array(ResponseHeader), | |
| method: Type.Optional(Type.Enum(ApiMethods)), | |
| status: Type.Number(), | |
| statusText: Type.Optional(Type.String()), | |
| time: Type.Optional(Type.Number()), | |
| redirectedUrl: Type.Optional(Type.String()), | |
| }); | |
| export const ResponseObject = Type.Object({ | |
| body: Type.String(), | |
| headers: Type.Array(ResponseHeader), | |
| method: Type.Optional(Type.Enum(ApiMethods)), | |
| status: Type.Integer({ minimum: 100, maximum: 599 }), | |
| statusText: Type.Optional(Type.String()), | |
| time: Type.Optional(Type.Number({ minimum: 0 })), | |
| redirectedUrl: Type.Optional(Type.String()), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/actions/local-sync/schemas.ts` around lines 148 - 156, Tighten
the ResponseObject schema by replacing the loose numeric types: change the
status property in ResponseObject from Type.Number() to Type.Integer({ minimum:
100, maximum: 599 }) to enforce valid HTTP status codes, and change time from
Type.Optional(Type.Number()) to Type.Optional(Type.Integer({ minimum: 0 })) so
time cannot be negative; keep the same property names and optional/required
semantics for ResponseObject, ResponseObject.status, and ResponseObject.time
when making the change.
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.
Summary by CodeRabbit