fail when the cli is given an unknown option - #339
Conversation
|
@avionicharshit-byte Good catch, and thanks for the fix! For warning vs hard failure: I think this should be a hard failure instead of a warning. Could you update the PR? |
11f16e8 to
912088c
Compare
|
@0xShug0 done , its a hard error now
the check also moved ahead of the run rather than after it , a typo now fails in 0.13s instead of running a full transcription first and failing at the end with the output already on stdout moving it earlier meant every option has to be read before the command branches , and two real bugs fell out of that.
the one case i could not make strict is the informational commands. built on macos arm64 with |
|
@avionicharshit-byte Thanks! PR merged. |
fixes #338. a misspelled option is currently dropped without a word, so
--bakend cudaruns on cpu and exits 0.find_arg,has_argandcollect_argsrecord the name they were asked for, andrequire_known_argsrefuses anything on the command line that nobody asked about. there is no second list of flags to keep in sync, a new option is recognised the moment its lookup is added.value-taking and boolean lookups are recorded separately because only the first kind consumes the next argument. without that split a typo sitting after a boolean flag gets swallowed as its value, and a value that happens to start with
--gets reported as an option. both were real, both are covered below.it is a hard error, and it runs before the model does any work. the message goes to stderr, stdout stays empty and the exit code is 1. checking before the run rather than after it means every option has to be read before the command branches, which is what most of this diff is: the output paths,
--input-format,--jsonand the batch and workflow options are now read up front and passed down, instead of being read halfway through or after the session has already finished.two real bugs fell out of doing that.
has_vad_chunk_optionshort circuited on||, so three--vad-chunk-*options were never looked up and got reported as unknown. seven more options are only read on a single code path, so they were rejected on every other one. both are fixed, and all 98 options the cli looks up are swept below with no false rejects.built at
e73c980on macos arm64,cmake -DAUDIOCPP_MODEL_SET=custom -DAUDIOCPP_MODELS=citrinet_asr --target audiocpp_cli:--task asr --family citrinet_asr --model <path> --audio speech.wav--task asr ... --bakend cudaunknown option: --bakend, exit 1, 0.13s, no stdout--task asr ... --sesion-option a=bunknown option: --sesion-option, exit 1--task asr ... --foo --barunknown options: --foo --bar, exit 1--task asr ... --metrics --foounknown option: --foo, exit 1--task asr ... --log-file --weird-looking-value--task asr ... --backend cpu --threads 2--list-loaders--list-loaders --bakend cudawarning: ignored option: --bakend, exit 0parity against main: stdout is byte for byte identical on every valid command tested, covering
--list-loaders,--list-loaders --json,--list-devices,--list-pipelines,--help,--help --task asr,--inspect, offline asr, offline vad, vad chunk outputs, batch audio dir, streaming and stdin pcm. output files written by--out-dir,--text-out,--words-out,--segments-out,--turns-out,--vad-chunks-outand--batch-manifest-outare identical too.what this does not do: the informational commands return before the rest of the options are read, so on
--help,--list-*and--inspectthe cli cannot tell a misspelling from an option that simply does not apply to that command. rejecting both would break anything that runs--list-devices --backend cudatoday, so those stay a warning and exit 0. making them strict as well needs the whole option set parsed up front before dispatch, which is a bigger change than this one and easy to do as a follow up.