-
Notifications
You must be signed in to change notification settings - Fork 63
fix: preserve wrapped CLI help in API docs #1826
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import assert from "node:assert/strict"; | ||
| import test from "node:test"; | ||
|
|
||
| import { normalizeParameterDescription, parseHelp } from "./extract-cli-model.mjs"; | ||
|
|
||
| test("parseHelp preserves wrapped argument and option descriptions", () => { | ||
| const parsed = parseHelp(`Usage: agentcore invoke [options] [prompt] | ||
|
|
||
| Invoke an agent. | ||
|
|
||
| Arguments: | ||
| prompt Prompt to send to the agent. Also accepts piped | ||
| stdin when no prompt is provided | ||
|
|
||
| Options: | ||
| --prompt-file <path> Read the prompt from a file (for long or | ||
| structured payloads that exceed shell limits) | ||
|
|
||
| Output | ||
| --json Output as JSON | ||
| `); | ||
|
|
||
| assert.equal( | ||
| parsed.args[0].description, | ||
| "Prompt to send to the agent. Also accepts piped stdin when no prompt is provided", | ||
| ); | ||
| assert.equal( | ||
| parsed.options[0].description, | ||
| "Read the prompt from a file (for long or structured payloads that exceed shell limits)", | ||
| ); | ||
| assert.equal(parsed.options[1].description, "Output as JSON"); | ||
| }); | ||
|
|
||
| test("normalizeParameterDescription expands model provider and LiteLLM fragments", () => { | ||
| assert.equal( | ||
| normalizeParameterDescription("bedrock, open_ai, or gemini"), | ||
| "The model provider. Valid values: `bedrock`, `open_ai`, or `gemini`.", | ||
| ); | ||
| assert.equal( | ||
| normalizeParameterDescription( | ||
| "Override LiteLLM API base URL (harness only, lite_llm) [non-interactive]", | ||
| ), | ||
| "The LiteLLM API base URL override for harness invocations. " + | ||
| "Available only with `lite_llm` in non-interactive mode.", | ||
| ); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,15 +55,71 @@ | |
| SCHEMA_VERSION = 1 | ||
|
|
||
|
|
||
| def normalize_style(text): | ||
| """Apply style-safe substitutions to generated prose.""" | ||
| if not text: | ||
| return "" | ||
| text = re.sub(r"\be\.g\.(?:,)?", "for example,", text, flags=re.IGNORECASE) | ||
| text = re.sub( | ||
| r"\bAWS Bedrock(?: AgentCore)? Code\s*Interpreter\b", | ||
| "AgentCore Code Interpreter", | ||
| text, | ||
| flags=re.IGNORECASE, | ||
| ) | ||
| text = re.sub(r"\bAWS Bedrock AgentCore\b", "Amazon Bedrock AgentCore", text) | ||
| text = re.sub(r"\bAWS Bedrock\b", "Amazon Bedrock", text) | ||
| text = re.sub(r"\bAgentCore Memory\b", "AgentCore memory", text) | ||
| text = re.sub(r"\bAgentCore Runtime\b", "AgentCore runtime", text) | ||
| text = text.replace( | ||
| "This feature is in preview and may change in future releases.", | ||
| "This feature is in preview and might change in future releases.", | ||
| ) | ||
| text = text.replace("validation will ensure", "validation ensures") | ||
| return re.sub(r"\bAWS\b", "{aws}", text) | ||
|
|
||
|
|
||
| def normalize_param_description(text): | ||
| """Normalize recurring parameter-description style issues.""" | ||
| text = normalize_style(text).strip() | ||
| exact_substitutions = { | ||
| "bedrock, open_ai, or gemini": ( | ||
| "The model provider. Valid values: `bedrock`, `open_ai`, or `gemini`." | ||
| ), | ||
| "Override LiteLLM API base URL (harness only, lite_llm) [non-interactive]": ( | ||
| "The LiteLLM API base URL override for harness invocations. " | ||
| "Available only with `lite_llm` in non-interactive mode." | ||
| ), | ||
| "Override LiteLLM additional params as a JSON object (harness only, lite_llm) " | ||
| "[non-interactive]": ( | ||
| "The additional LiteLLM parameters, as a JSON object, for harness invocations. " | ||
| "Available only with `lite_llm` in non-interactive mode." | ||
| ), | ||
| } | ||
| if text in exact_substitutions: | ||
| return exact_substitutions[text] | ||
| substitutions = ( | ||
| (r"^Optional\b", "An optional"), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 This global rewrite breaks valid plural and mass-noun descriptions from the renderer's other supported sources. I reproduced |
||
| (r"^(?:\{aws\}|AWS)\s+region\b", "The {aws} Region"), | ||
| (r"^id of\b", "The ID of"), | ||
| (r"^Behaviour\b", "The behavior"), | ||
| (r"^Behavior\b", "The behavior"), | ||
| (r"^Memory resource ID\b", "The memory resource ID"), | ||
| (r"^Strategy name\b", "The name of the memory strategy"), | ||
| (r"^Strategy ID\b", "The ID of the memory strategy"), | ||
| ) | ||
| for pattern, replacement in substitutions: | ||
| text = re.sub(pattern, replacement, text, count=1, flags=re.IGNORECASE) | ||
| return text | ||
|
|
||
|
|
||
| def esc(text): | ||
| """Escape AsciiDoc-significant characters in inline text.""" | ||
| if not text: | ||
| return "" | ||
| # Guard the couple of chars that start AsciiDoc markup in running prose. | ||
| return ( | ||
| text.replace("|", "\\|") | ||
| .replace("{", "\\{") | ||
| ) | ||
| marker = "\0AWS_ENTITY\0" | ||
| text = normalize_style(text).replace("{aws}", marker) | ||
| return text.replace("|", "\\|").replace("{", "\\{").replace(marker, "{aws}") | ||
|
|
||
|
|
||
| # Match markdown code fences that may be indented (reST/Google docstrings often | ||
|
|
@@ -116,7 +172,7 @@ def render_params(params, out): | |
| req = "" if p.get("required") else " _(optional)_" | ||
| typ = f"`{p['type']}`" if p.get("type") else "" | ||
| out.append(f"`{p['name']}`{req} {typ}::") | ||
| out.append(esc(p.get("description", "")) or "_No description._") | ||
| out.append(esc(normalize_param_description(p.get("description", ""))) or "_No description._") | ||
| out.append("") | ||
|
|
||
|
|
||
|
|
||
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.
🟡 The regression test passes with
node --test, but no repository test target runs it: the unit project only discoverssrc/**/*.test.ts(x), and the package test scripts invoke Vitest. I confirmed Vitest reports this file as “No test files found.” Please wire it into a CI-invoked script/workflow or convert/move it into the discovered suite so this regression remains enforced.