-
Notifications
You must be signed in to change notification settings - Fork 10
auto generated cli #384
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
auto generated cli #384
Conversation
| "--quiet", | ||
| action="store_true", | ||
| help="If set, only errors will be printed.", | ||
| ) |
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.
Missing CLI arguments for dataset creation workflow
The --dataset-jsonl, --dataset-builder, and --dataset-display-name arguments were removed from the CLI but the code in create_rft.py still expects them via getattr(args, "dataset_jsonl", None) and getattr(args, "dataset_display_name", None). These are workflow-specific arguments (not SDK parameters) for creating datasets from local JSONL files. Users attempting to use --dataset-jsonl will get "unrecognized arguments" errors, and the dataset creation workflow from local files is broken. The comment at line 405-406 acknowledges that workflow controls must be maintained manually, but these arguments were not added.
Additional Locations (1)
| prefix = name.replace("_", "-") | ||
| field_kebab = field_name.replace("_", "-") | ||
| flag_name = f"--{prefix}-{field_kebab}" | ||
| flags = [flag_name] + aliases.get(f"{name}.{field_name}", []) + [f"--{field_kebab}"] |
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.
Short aliases for nested fields may cause conflicts
The add_args_from_callable_signature function automatically adds --{field_kebab} as a short alias for every nested TypedDict field. If two different TypedDict parameters in the SDK signature have fields with the same name (e.g., both training_config and inference_parameters having a temperature field), argparse will raise an error about conflicting option strings when the parser is built. This fragile pattern depends on the SDK never having overlapping field names across nested configs.
| if nested: | ||
| sdk_kwargs[name] = nested | ||
| elif args_dict.get(name) is not None: | ||
| sdk_kwargs[name] = args_dict[name] |
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.
SDK kwargs overwrite normalized values with raw args
The _create_rft_job function correctly initializes sdk_kwargs with normalized evaluator_resource_name and dataset_resource values from function parameters. However, the loop that builds nested SDK kwargs (lines 636-652) also checks for top-level parameters like evaluator and dataset in args_dict. Since these are parameters in the SDK signature and are added as CLI flags via add_args_from_callable_signature, user-provided raw values (e.g., --evaluator my-eval) will overwrite the correctly-normalized full resource names (e.g., accounts/{id}/evaluators/my-eval). This causes the SDK call to receive short IDs instead of full resource names, likely causing API failures.
Note
Auto-generates
create rftCLI flags from the Fireworks SDK and switches RFT job creation to the SDK with stricter arg parsing and new tests.Modernizes the
create rftCLI by introspecting the Fireworks SDK and tightening argument handling.create rftflags fromFireworks().reinforcement_fine_tuning_jobs.create(with aliases, help overrides, and field skipping) via newcli_commands/utils.add_args_from_callable_signature--dry-runnow prints planned SDK kwargs--source-job,--quiet) while keeping validator/docker flagsfireworks-ai==1.0.0a18and update lockfile; remove REST helper fromfireworks_rft.pyWritten by Cursor Bugbot for commit 665ea5e. This will update automatically on new commits. Configure here.