-
Notifications
You must be signed in to change notification settings - Fork 1
Feat context-help: prioritize textExpression when present and open preview; fallback to preview-only when empty #59
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
…review; fallback to preview-only when empty
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function extractEmbeddedContextExpression(content: string): { | ||
| previewContent: string; | ||
| textExpression?: string; | ||
| } { | ||
| const jsonPattern = /\{[\s\S]*\}\s*$/; | ||
| const match = jsonPattern.exec(content); | ||
| if (!match || match.index === undefined) { | ||
| return { previewContent: content }; | ||
| } | ||
|
|
||
| const previewWithoutJson = content.slice(0, match.index).replace(/\s+$/, ""); | ||
| const jsonText = content.slice(match.index).trim(); | ||
|
|
||
| try { | ||
| const parsed = JSON.parse(jsonText); | ||
| if (!isRecord(parsed)) { | ||
| return { previewContent: previewWithoutJson }; | ||
| } | ||
|
|
||
| const { status, textExpression } = parsed as ResolveContextExpressionResponse; | ||
| if (typeof status !== "string" || status.toLowerCase() !== "success") { | ||
| return { previewContent: previewWithoutJson }; | ||
| } | ||
|
|
||
| if (typeof textExpression !== "string") { | ||
| return { previewContent: previewWithoutJson }; | ||
| } | ||
|
|
||
| if (!textExpression.trim()) { | ||
| return { previewContent: previewWithoutJson }; | ||
| } | ||
|
|
||
| return { previewContent: previewWithoutJson, textExpression }; | ||
| } catch (_error) { | ||
| return { previewContent: content }; |
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.
Make trailing JSON extraction ignore braces in documentation body
The new extractEmbeddedContextExpression looks for the trailing JSON block with /\{[\s\S]*\}\s*$/ and uses the first match returned by exec. Because the pattern is greedy and starts at the first { in the string, any documentation text that contains curly braces before the appended JSON causes jsonText to include the entire body plus the JSON footer. Parsing then fails and the function returns the original string, so the embedded textExpression is never applied and the raw JSON remains visible in the preview. This regression occurs whenever the documentation itself includes braces (e.g. code samples). The extractor should target the last JSON object explicitly (e.g. by searching for the last { after the final newline) instead of the first {.
Useful? React with 👍 / 👎.
Handle ResolveContextExpression responses that may include both:
textExpression.Behavior:
textExpressionis non-empty and JSON payload is present: replace the line usingtextExpressionand open the preview with the doc string.textExpressionis empty: keep current behavior (show only preview or surface error).Also adds guards for mixed responses and maintains existing error handling when neither path applies.