Repository navigation
Conversation
|
Hi, thanks for your contribution. Could we include the fix from 27d58c5 in PR #538, or link a follow-up before merging? The CLI exposes input_dir, but cli_text2model_train does not accept it, so every text2model train invocation currently fails with TypeError before the helper runs. This predates PR #538, but it makes the command unusable. Please add the parameter and regression coverage, or track the fix explicitly. |
|
Thanks — I reproduced this on the previous #538 head. The I've added the minimal The updated PR CI is passing. Broader Text2Model cache/runtime-path work will remain a separate follow-up. |
|
LGTM, sincerely thanks! |
Summary
Make the public
pdf2modelandtext2modelTyper commands return a nonzero process exit status when their helper reports operational failure viaFalse.This PR also fixes the existing
text2model trainCLI/helper mismatch where the public command passesinput_dirbutcli_text2model_train()did not accept it.Problem
The four public PDF/Text init/train wrappers call helpers that use boolean success/failure contracts. Before this patch, a helper could print a failure, return
False, and still leave the public CLI process at exit status 0.This is especially unsafe for shell, CI, and orchestration callers that use the process exit code as the workflow success signal.
There was also a pre-existing issue in the Text2Model train path: the public CLI passes
input_dirtocli_text2model_train(), but the helper did not accept that argument and instead hard-codedinput_dir = "./". As a result,text2model traincould fail withTypeErrorbefore the helper body ran.Root cause
The wrappers invoked the helper but ignored its boolean return value. Exceptions already mapped to nonzero exits, but normal
Falsereturns were not interpreted at the public CLI boundary.For Text2Model, the public CLI and helper signature were also out of sync for
input_dir.Fix
typer.Exit(code=1)when the helper result is false;typer.Exitoutside the broad exception block to avoid duplicate error reporting;input_dirtocli_text2model_train()and remove the internal hard-codedinput_dir = "./"assignment.Regression tests
Added
test/cpu_only/test_cli_helper_failure_exit.pycovering:False→ nonzero public exit;True→ exit 0;The helper test doubles now use explicit compatible signatures instead of a permissive variadic helper.
Added
test/cpu_only/test_text2model_input_dir_boundary.pyto exercise the public Typer CLI → realcli_text2model_train()boundary and verify that the helper accepts and uses the publicinput_dirargument.Targeted validation:
git diff --check: passed;py_compile: passed;Python testsCI: passed.Additional validation
Repeated controlled validation covered the four public commands across helper
True,False, and exception outcomes with no counterexample.The Text2Model boundary regression also confirms that the helper body is reached after the signature fix and that the existing default behavior remains compatible.
Limitations
This PR is intentionally scoped to public CLI failure propagation and the Text2Model
input_dirsignature mismatch.It does not change detector behavior, PDF KBC/VQA wiring, broader cache-path propagation, stale-artifact handling, or the rest of the Text2Model data-preparation path.
The available environment did not run real PDF/Text model training E2E.