Summary
Implements CLI arguments for specifying custom template and evaluation config files as requested in Issue #201.
Changes Made
- Added
--template and --evaluation-config command-line arguments
- Modified argument parsing in both
main.py and entry.py
- Maintained backward compatibility with existing default file paths
- Fixed file path resolution for improved reliability
Testing
- Tested with custom config paths
- Verified backward compatibility with default behavior
- Confirmed proper error handling for invalid paths
Closes #201
Hi @gitttjonzzz, thanks for the neat PR, but I see some ai comments that you may want to modify. Please check my review comments
PR Code Suggestions ✨
Latest suggestions up to f880756
| Category | Suggestion                                                                  | Impact |
| Possible issue |
Fix CLI args key mismatch
The new CLI keys in main.py use dest='template_file'/config_file/evaluation_file,
but here the code reads templateFile/configFile/evaluationFile. This mismatch will
cause the CLI options to be ignored. Align the argument names with the dest keys.
src/entry.py [34-36]
-template_path = Path(args["templateFile"]) if args.get("templateFile") else input_dir / "template.json"
-config_path = Path(args["configFile"]) if args.get("configFile") else input_dir / "config.json"
-evaluation_path = Path(args["evaluationFile"]) if args.get("evaluationFile") else input_dir / "evaluation.json"
+template_path = Path(args["template_file"]) if args.get("template_file") else input_dir / "template.json"
+config_path = Path(args["config_file"]) if args.get("config_file") else input_dir / "config.json"
+evaluation_path = Path(args["evaluation_file"]) if args.get("evaluation_file") else input_dir / "evaluation.json"
Suggestion importance[1-10]: 9
__
Why: Correctly identifies a breaking mismatch between argparse dest keys (template_file, etc.) and accessed keys (templateFile, etc.), which would prevent CLI overrides from working; the improved code aligns with main.py.
| High
|
Use correct CLI arg keys
args contains the lowercase keys (config_file) from argparse dest; accessing
configFile will always be False. Update all such lookups (configFile, templateFile,
evaluationFile) in process_dir to their lowercase underscore forms to ensure CLI
overrides work.
src/entry.py [107-114]
-if args.get("configFile"):
+if args.get("config_file"):
# Use CLI-specified config file
- config_path = Path(args["configFile"])
+ config_path = Path(args["config_file"])
if os.path.exists(config_path):
tuning_config = open_config_with_defaults(config_path)
else:
logger.error(f"Specified config file does not exist: {config_path}")
raise Exception(f"Config file not found: {config_path}")
Suggestion importance[1-10]: 9
__
Why: This fixes the same critical key mismatch inside process_dir, ensuring CLI-specified config overrides are honored; aligns with main.py dest names and prevents silent no-ops.
| High
|
Restore missing return value
entry_point previously returned process_dir(...), but the new code drops the return
value. If callers rely on it (e.g., for exit status), this is a regression. Return
the result of process_dir(...) to preserve behavior.
src/entry.py [52-59]
# Process the directory
-process_dir(
+return process_dir(
input_dir,
input_dir,
args,
template=None,
tuning_config=CONFIG_DEFAULTS,
evaluation_config=None,
)
Suggestion importance[1-10]: 4
__
Why: Returning the process_dir result may preserve prior behavior, but the PR seems to intentionally change flow and there’s no evidence callers use the return; moderate impact and somewhat speculative.
| Low
|
Previous suggestions
Suggestions up to commit f880756
| Category | Suggestion                                                                  | Impact |
| Possible issue |
Use Namespace attributes directly
The code treats args like a dict, but parse_args() returns a Namespace. Access CLI
flags using attribute syntax (args.template_file, args.config_file,
args.evaluation_file) or convert args to a dict via vars(args). This will prevent
TypeError at runtime.
main.py [114-119]
-if args.get("template_file"):
- logger.info(f"Using custom template file: {args['template_file']}")
-if args.get("config_file"):
- logger.info(f"Using custom config file: {args['config_file']}")
-if args.get("evaluation_file"):
- logger.info(f"Using custom evaluation file: {args['evaluation_file']}")
+if args.template_file:
+ logger.info(f"Using custom template file: {args.template_file}")
+if args.config_file:
+ logger.info(f"Using custom config file: {args.config_file}")
+if args.evaluation_file:
+ logger.info(f"Using custom evaluation file: {args.evaluation_file}")
Suggestion importance[1-10]: 9
__
Why: The code incorrectly treats args as a dict instead of a Namespace, which will cause a runtime TypeError; using attribute access is necessary for correctness.
| High
|
Fix mismatched CLI attribute names
The dest names in add_argument are snake_case (template_file, config_file,
evaluation_file), but this block uses camelCase keys. Replace dict-style access with
the correct attributes (args.template_file, etc.) so the CLI flags work as intended.
src/entry.py [34-36]
-template_path = Path(args["templateFile"]) if args.get("templateFile") else input_dir / "template.json"
-config_path = Path(args["configFile"]) if args.get("configFile") else input_dir / "config.json"
-evaluation_path = Path(args["evaluationFile"]) if args.get("evaluationFile") else input_dir / "evaluation.json"
+template_path = Path(args.template_file) if args.template_file else input_dir / "template.json"
+config_path = Path(args.config_file) if args.config_file else input_dir / "config.json"
+evaluation_path = Path(args.evaluation_file) if args.evaluation_file else input_dir / "evaluation.json"
Suggestion importance[1-10]: 8
__
Why: The code uses incorrect camelCase keys (templateFile etc.) that don't match the dest names, so CLI flags won't be recognized without this fix.
| Medium
|
Udayraj123
