feat: generate SDK for TypeScript and Python #623
Conversation
…sive tests - Fix var=true boolean flag typed as Optional[bool] in Python - Fix -- separator ordering for double_dash=required args - Fix Python var flag default type mismatch - Align SDK tests across Python and TypeScript - Add ~30 new Python tests and ~14 new TypeScript tests - Add compile/import validation tests for Python and TypeScript - Mark Rust SDK as coming soon in docs - Add Typer and Click integration links
- Python: add client_edge_cases, config_and_flag_edge_cases, double_dash_automatic - TypeScript: add config_boolean_default_false, config_string_with_default, example_without_lang, flag_edge_cases, global_flags_flags_only, double_dash_automatic
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #623 +/- ##
==========================================
+ Coverage 78.94% 79.31% +0.37%
==========================================
Files 49 56 +7
Lines 7284 10487 +3203
Branches 7284 10487 +3203
==========================================
+ Hits 5750 8318 +2568
- Misses 1147 1409 +262
- Partials 387 760 +373 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Remove Rust SDK module, language enum variant, CLI option, and all snapshot files. Rust SDK will be re-added in a future update.
f9986cc to
44a8480
Compare
|
Warning Gemini is experiencing higher than usual traffic and was unable to create the review. Please try again in a few hours by commenting |
Greptile SummaryThis PR introduces a
Confidence Score: 5/5The generated output is safe; string escaping, reserved-word sanitization, and choice-collision detection look correct across both targets. Escape helpers are applied consistently, sanitize_ident covers exec/runner/buildFlagArgs, global flags are correctly deduplicated in TypeScript interface bodies and Python _build_flag_args loops, and from future import annotations is present in all three Python files. Snapshot and integration compile tests provide strong regression coverage. No files require special attention. Important Files Changed
Reviews (20): Last reviewed commit: "fix(sdk): validate count-flag default as..." | Re-trigger Greptile |
- P2: re-export CliResult/CliRunner from Python __init__.py and CliResult from TypeScript index.ts - P2: escape */ in JSDoc and triple-quote in Python docstrings to prevent premature termination from user-supplied spec text - P2: fix unreachable "True"/"False"/"None" in sanitize_py_ident (heck::AsSnakeCase lowercases before match) - P2: remove incorrect await from docs synchronous API example
Replace spawnSync with spawn + Promise wrapper in runtime.ts. All exec() methods now return Promise<CliResult> and are marked async, which is more idiomatic for Node.js and avoids blocking the event loop on subprocess calls. Update docs examples to use await. Update all TypeScript snapshots.
059c78e to
5b958fc
Compare
…types
- P1: escape backslashes and double quotes in Python string defaults
(config props, arg defaults, flag defaults, VERSION/ABOUT/AUTHOR,
bin_path). Add escape_py_string() and escape_ts_string() helpers.
- P2: export CliError from TypeScript index.ts
- P2: stop emitting empty XxxFlags extends GlobalFlags {} interfaces
in types.ts when subcommands only have global flags with no local
flags — client.ts already uses GlobalFlags directly in this case
- Python: escape choice values, cmd path elements, flag_arg_name, negate flag names in string literals; escape alias docstrings - TypeScript: escape choice values in type unions, cmd path elements in subcmd_path, bin_name in constructor, flag_arg_name, negate flag names in string literals Note: flag short names (char type) cannot contain " or \ so they do not need escaping.
…gment - Python: render multi-part exec docstrings as proper multiline docstrings instead of single-line with embedded \n - docs: fix broken sentence from Rust SDK removal (child_process.spawn / not -> child_process.spawn, not)
20bf555 to
6c483d9
Compare
Python render_command_types was generating {name}Flags dataclasses
for subcommands that only inherit global flags (no local flags).
Since client.py uses GlobalFlags directly for these cases, the
generated dataclass was dead code. Now only emit Flags dataclass
when there are local visible flags, matching the TypeScript fix.
1f8596e to
dd222a2
Compare
| #[cfg(feature = "docs")] | ||
| pub mod docs; | ||
| pub mod parse; | ||
| #[cfg(feature = "sdk")] |
There was a problem hiding this comment.
Why add a feature?
I found it relatively independent and make it a feature. Should we remove it?
…ze newlines in Python comments
📝 WalkthroughWalkthroughThis PR introduces comprehensive SDK generation for TypeScript and Python, allowing developers to generate type-safe client libraries from CLI usage specs. The implementation spans CLI integration, core infrastructure, language-specific generators, documentation, and validation tests. ChangesSDK Generation Feature
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Generate as generate::Sdk
participant SdkCore as sdk::generate
participant TypeScript as typescript::generate
participant Python as python::generate
participant Output as Output Files
User->>Generate: generate sdk --file spec.yaml --language typescript --output ./sdk
Generate->>Generate: parse spec, validate language
Generate->>SdkCore: generate(&spec, &SdkOptions)
alt language == TypeScript
SdkCore->>TypeScript: generate(&spec, &opts)
TypeScript->>TypeScript: render types.ts, client.ts, runtime.ts, index.ts
TypeScript-->>SdkCore: SdkOutput
else language == Python
SdkCore->>Python: generate(&spec, &opts)
Python->>Python: render types.py, client.py, runtime.py, __init__.py
Python-->>SdkCore: SdkOutput
end
SdkCore-->>Generate: SdkOutput
Generate->>Generate: ensure output directory
Generate->>Output: write each file
Output-->>User: SDK files written to ./sdk
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
1b024e3 to
384176d
Compare
|
@jdx ready for review! sorry for being late! |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
lib/tests/sdk_compile.rs (2)
164-167: ⚡ Quick winMove npm availability check earlier.
The npm existence check is performed after generating the SDK, creating the temp directory, and writing files. If npm is not available, this work is wasted. Consider moving this check to the start of the test alongside the
npx_tsc_available()check for better efficiency.♻️ Suggested reordering
Move lines 164-167 to after line 148:
#[test] fn test_typescript_sdk_typechecks() { if !npx_tsc_available() { eprintln!("Skipping TypeScript SDK typecheck test - tsc not available via npx"); return; } if !tool_exists("npm") { eprintln!("Skipping TypeScript SDK typecheck test - npm not found"); return; } let spec = full_spec(); // ... rest of test🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/tests/sdk_compile.rs` around lines 164 - 167, Move the npm availability check earlier in the test to avoid doing work when npm is missing: in the test_typescript_sdk_typechecks function, place the tool_exists("npm") check right after the existing npx_tsc_available() check (so both precondition checks run before creating the temp dir, generating the SDK, or writing files), and keep the same eprintln! message and early return behavior.
190-202: Note: tsconfig requires TypeScript 5.0+.The
"moduleResolution": "bundler"option was introduced in TypeScript 5.0. Sincemise.tomlspecifies"latest"for the TypeScript version, this should work correctly. However, if developers use older TypeScript versions locally, this test may fail.Consider adding a comment documenting this requirement, or explicitly checking the TypeScript version if broader compatibility is needed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/tests/sdk_compile.rs` around lines 190 - 202, The embedded tsconfig string assigned to tsconfig uses "moduleResolution": "bundler" which requires TypeScript 5.0+; update the test to either add an inline comment near the tsconfig variable noting the TypeScript 5.0+ requirement or add a runtime version check before running the test (e.g., detect the installed tsc version and skip the test if <5.0) so local older TypeScript installations don't cause spurious failures; reference the tsconfig variable in sdk_compile.rs and implement the chosen mitigation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/assets/fig.ts`:
- Line 465: Update the --file flag description to document stdin support: change
the description string (currently "A usage spec taken in as a file") to mention
that "-" can be used to read from stdin (consistent with other generate
commands). Reference the --file flag description in cli/assets/fig.ts and the
helper file_or_spec which accepts "-" as the stdin sentinel so users know they
can pass "-" to read from standard input.
In `@cli/assets/usage.1`:
- Line 325: Update the --file flag description for the "A usage spec taken in as
a file" entry to mention stdin support (that "-" will read from stdin) so it
matches other generate commands; locate the usage text for the usage spec entry
in the cli/assets/usage.1 content and append a short phrase like 'use "-" to
read from stdin' to the --file flag description to reflect existing
implementation behavior.
In `@cli/src/cli/generate/sdk.rs`:
- Line 12: Update the doc comment on the usage spec in
cli/src/cli/generate/sdk.rs (the line currently "/// A usage spec taken in as a
file") to document that passing "-" will read the spec from stdin, matching the
behavior of generate::file_or_spec; keep the wording concise and reference "-"
as the stdin convention so it aligns with other generate commands' docs.
In `@cli/usage.usage.kdl`:
- Line 139: Update the help text for the flag declaration flag "-f --file" in
the usage command to explicitly note it accepts "-" / stdin (e.g., "A usage spec
taken in as a file or '-' to read from stdin") so it matches other generate
commands and actual implementation behavior; change the help string in the flag
"-f --file" declaration accordingly.
In `@docs/cli/reference/commands.json`:
- Line 753: Update the usage help text to document stdin support: modify the
"help" string for the usage spec (the JSON entry with "help": "A usage spec
taken in as a file") to mention that stdin is accepted (e.g., "A usage spec
taken from a file or from stdin"), then update the source KDL spec
(cli/usage.usage.kdl) accordingly and regenerate the commands.json so the change
is reflected in the generated output.
---
Nitpick comments:
In `@lib/tests/sdk_compile.rs`:
- Around line 164-167: Move the npm availability check earlier in the test to
avoid doing work when npm is missing: in the test_typescript_sdk_typechecks
function, place the tool_exists("npm") check right after the existing
npx_tsc_available() check (so both precondition checks run before creating the
temp dir, generating the SDK, or writing files), and keep the same eprintln!
message and early return behavior.
- Around line 190-202: The embedded tsconfig string assigned to tsconfig uses
"moduleResolution": "bundler" which requires TypeScript 5.0+; update the test to
either add an inline comment near the tsconfig variable noting the TypeScript
5.0+ requirement or add a runtime version check before running the test (e.g.,
detect the installed tsc version and skip the test if <5.0) so local older
TypeScript installations don't cause spurious failures; reference the tsconfig
variable in sdk_compile.rs and implement the chosen mitigation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64c1e291-016d-4d1e-8e11-07e9123e0c39
⛔ Files ignored due to path filters (71)
Cargo.lockis excluded by!**/*.locklib/src/sdk/python/snapshots/usage__sdk__python__tests__python_boolean_flag_default_false.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_choice_collision.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_client.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_client_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_config_all_optional.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_config_and_flag_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_config_boolean_default_false.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_config_props.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_config_string_with_default.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_count_flag_build.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_deep_nesting.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_double_dash_automatic.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_example_without_lang.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_exec_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_flag_edge_cases-2.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_flag_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_flag_with_choices.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_flag_with_env.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_flags_only_subcommand.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_full_feature_client.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_full_feature_types.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_global_flags_flags_only.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_global_repeatable_flags.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_hyphenated_subcommands.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_init.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_minimal.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_multiple_aliases.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_negate_flag_build.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_optional_arg_empty_flags.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_optional_variadic_arg.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_package_name_override.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_required_flag_type.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_runtime.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_types.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_var_value_flag_with_default.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__choice_collision.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__deep_nesting.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__flags_only_subcommand.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__full_feature_client.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__full_feature_types.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__hyphenated_subcommands.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__minimal_spec.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__package_name_override.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_arg_defaults.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_boolean_flag_default_false.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_client.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_client_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_config_all_optional.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_config_and_flag_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_config_boolean_default_false.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_config_string_with_default.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_count_flag_build.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_double_dash_automatic.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_example_without_lang.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_flag_edge_cases-2.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_flag_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_flag_with_choices.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_flag_with_env.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_global_flags_flags_only.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_global_repeatable_flags.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_index.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_multiple_aliases.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_negate_flag_build.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_optional_arg_empty_flags.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_optional_variadic_arg.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_required_flag_type.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_runtime.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_types.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_var_value_flag_with_default.snapis excluded by!**/*.snapmise.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
cli/assets/fig.tscli/assets/usage.1cli/src/cli/generate/mod.rscli/src/cli/generate/sdk.rscli/usage.usage.kdldocs/.vitepress/config.mtsdocs/cli/reference/commands.jsondocs/cli/reference/generate.mddocs/cli/reference/generate/sdk.mddocs/cli/reference/index.mddocs/cli/sdk.mddocs/spec/index.mdlib/Cargo.tomllib/src/lib.rslib/src/sdk/mod.rslib/src/sdk/python/mod.rslib/src/sdk/python/runtime.rslib/src/sdk/typescript/mod.rslib/src/sdk/typescript/runtime.rslib/src/sdk/typescript/types.rslib/src/sdk/typescript/wrappers.rslib/src/spec/mod.rslib/tests/sdk_compile.rsmise.toml
Summary by CodeRabbit
New Features
generate sdkcommand to generate type-safe SDK client libraries from usage specs in TypeScript and PythonDocumentation