-
Notifications
You must be signed in to change notification settings - Fork 445
fix: add JSON fallback for spec generation with custom API endpoints #514
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
fix: add JSON fallback for spec generation with custom API endpoints #514
Conversation
Fixes spec generation failure when using custom API endpoints (e.g., GLM proxy) that don't support structured output. The AI returns JSON instead of XML, but the fallback parser only looked for XML tags. Changes: - escapeXml: Handle undefined/null values gracefully (converts to empty string) - generate-spec: Add JSON extraction fallback when XML tags aren't found - Reuses existing extractJson() utility (already used for Cursor models) - Converts extracted JSON to XML using specToXml() Closes AutoMaker-Org#510 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 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 |
Summary of ChangesHello @Seonfx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a critical fix for spec generation failures that occur when using custom AI API endpoints, such as GLM proxy, which may return plain JSON instead of the expected XML. It enhances the system's robustness by adding a JSON extraction fallback and converting it to XML, while also improving the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a helpful fallback mechanism for spec generation, allowing it to handle JSON responses from custom API endpoints that don't produce the expected XML. The changes are logical and well-structured. I've made a couple of suggestions:
- In
app-spec-format.ts, I've proposed a more concise way to handlenullandundefinedvalues inescapeXml. - More importantly, in
generate-spec.ts, I've identified a potential runtime error wherespecToXmlcould crash if the extracted JSON doesn't match theSpecOutputschema. I've suggested adding a validation step to prevent this, making the new fallback logic more robust.
These changes will improve code quality and prevent potential crashes. Otherwise, the implementation looks solid.
| logger.warn('⚠️ No XML tags found, attempting JSON extraction...'); | ||
| const extractedJson = extractJson<SpecOutput>(responseText, { logger }); | ||
|
|
||
| if (extractedJson) { |
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.
While extractJson successfully parses a JSON object, it doesn't validate its structure against the SpecOutput type. This could lead to a runtime error in specToXml if a required array property (like technology_stack) is missing, as it would attempt to call .map on undefined.
To prevent this, you should add validation to ensure the extracted JSON conforms to the expected SpecOutput structure before passing it to specToXml. A simple check for the presence and type of required fields would make this fallback much more robust.
| if (extractedJson) { | |
| if (extractedJson && | |
| typeof extractedJson.project_name === 'string' && | |
| typeof extractedJson.overview === 'string' && | |
| Array.isArray(extractedJson.technology_stack) && | |
| Array.isArray(extractedJson.core_capabilities) && | |
| Array.isArray(extractedJson.implemented_features)) { |
| if (str === undefined || str === null) { | ||
| return ''; | ||
| } | ||
| return String(str) |
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.
For better conciseness and to leverage TypeScript's type narrowing, you can simplify this check. Using str == null checks for both undefined and null. After this check, TypeScript knows str is a string, so the String() wrapper is not necessary.
| if (str === undefined || str === null) { | |
| return ''; | |
| } | |
| return String(str) | |
| if (str == null) { | |
| return ''; | |
| } | |
| return str |
|
If u could adress pr comments before i review it |
- Simplify escapeXml() using 'str == null' check (type narrowing) - Add validation for extracted JSON before passing to specToXml() - Prevents runtime errors when JSON doesn't match SpecOutput schema 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
done. |
Summary
Fixes spec generation failure when using custom API endpoints (e.g., GLM proxy) that don't support structured output. The AI returns JSON instead of XML, but the fallback parser only looked for XML tags.
Root Cause
When
ANTHROPIC_BASE_URLis set to a custom endpoint:json_schema) is disabled (correct - custom endpoints may not support it)<project_specification>XML tagsescapeXml()crashed onundefined/nullvaluesChanges
apps/server/src/lib/app-spec-format.tsescapeXml()to handleundefined/nullvalues gracefully (converts to empty string)String()wrapper for type safetyapps/server/src/routes/app-spec/generate-spec.tsextractJson()utility (already used for Cursor models)specToXml()Related Issue
Closes #510
Testing
🤖 Generated with Claude Code