Skip to content

Conversation

@xzrderek
Copy link
Contributor

@xzrderek xzrderek commented Dec 22, 2025

Note

Auto-generates create rft CLI flags from the Fireworks SDK and switches RFT job creation to the SDK with stricter arg parsing and new tests.

Modernizes the create rft CLI by introspecting the Fireworks SDK and tightening argument handling.

  • Auto-generates create rft flags from Fireworks().reinforcement_fine_tuning_jobs.create (with aliases, help overrides, and field skipping) via new cli_commands/utils.add_args_from_callable_signature
  • Replaces manual REST payload construction with Fireworks SDK calls; --dry-run now prints planned SDK kwargs
  • Stricter argument parsing: fail fast on unknown flags; minor help/metavar cleanups
  • Adds workflow-only flags (--source-job, --quiet) while keeping validator/docker flags
  • Dependencies: add fireworks-ai==1.0.0a18 and update lockfile; remove REST helper from fireworks_rft.py
  • Extensive tests cover unknown-flag failure, help output, and correct SDK kwargs mapping for nested configs

Written by Cursor Bugbot for commit 665ea5e. This will update automatically on new commits. Configure here.

@xzrderek xzrderek requested a review from dphuang2 December 22, 2025 06:33
@xzrderek xzrderek requested a review from dphuang2 December 22, 2025 21:46
"--quiet",
action="store_true",
help="If set, only errors will be printed.",
)
Copy link

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)

Fix in Cursor Fix in Web

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}"]
Copy link

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.

Fix in Cursor Fix in Web

@xzrderek xzrderek enabled auto-merge (squash) December 22, 2025 22:14
@xzrderek xzrderek merged commit 5fc4592 into main Dec 22, 2025
17 checks passed
@xzrderek xzrderek deleted the derekx/auto-generated-cli branch December 22, 2025 22:20
if nested:
sdk_kwargs[name] = nested
elif args_dict.get(name) is not None:
sdk_kwargs[name] = args_dict[name]
Copy link

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.

Additional Locations (1)

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants