fix(cli): single model-command owner; fail loud on defaults and prompt input - #202
Merged
marlon-costa-dc merged 4 commits intoSep 26, 2026
Merged
Conversation
Delete the duplicate u.Cli.build_model_command Builder (and its SignatureCarrier) that passed raw field annotations to Typer, plus the u.Cli.derive_model copy of cli.derive_model. No workspace member consumes either; u.Cli.model_source_data stays as the primitive cli.derive_model uses. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Remove the CLI_SAFE_EXCEPTIONS catch-and-normalize from confirm and the _guarded/_fatal helpers behind prompt, prompt_choice and prompt_password. A reader exception now propagates unchanged; cancellation (KeyboardInterrupt) and end of input (EOFError) stay declared r.fail outcomes and carry the exception. prompt_choice never read input, so its message parameter, used only by the removed failure log, is dropped. Tests assert propagation through the public service with scripted input ports. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
u.Cli.field_default no longer catches the default-source validation error and returns None. None stays the typed absence of a default; any other unrepresentable default now fails cli.model_command at build: the validation error escapes unchanged, and a validated default no Typer option carries raises TypeError naming the field. Structured defaults keep the JSON-option path. Drop the prompt failure-format constants left unused by the prompt fail-loud change. make fmt normalized two option test files. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… test Build the settings model before pytest.raises so the block holds exactly one invocation that can raise (SonarCloud python:S5778 on PR #202). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
marlon-costa-dc
added a commit
that referenced
this pull request
Sep 26, 2026
…g-free process tests (#203) Follow-up to #202. It resolves two rule violations: the remaining catch-and-normalize sites (fail loud), and tests that measured host timing instead of behaviour. ## 1. Remaining `CLI_SAFE_EXCEPTIONS` catches This is the same violation class #202 removed from prompts. `CLI_SAFE_EXCEPTIONS` has no consumers left, so it is deleted. | Site | Before | After | | --- | --- | --- | | `u.Cli.cmd_validate_settings` (`_utilities/cmd.py`) | `ValueError`/`TypeError`/`KeyError` became `r.fail("Settings validation failed: …")` | No catch. Any exception escapes with its cause. | | `u.Cli.tables_resolve_config` (`_utilities/tables.py`) | Every listed type became `r.fail` with message only | Only the model's `ValidationError` (the declared outcome) becomes `r.fail(…, exception=exc)`. Everything else escapes. | | `u.Cli.tables_render` (`_utilities/tables.py`) | Renderer errors became `r.fail_op("Table formatting", …)` | No catch. Renderer exceptions escape. | | `u.Cli.process_mapping` (`_utilities/validation.py`) | `skip` mode swallowed exceptions into a debug log and `collect` aggregated them | Removed together with `t.Cli.MappingProcessor`. Nothing in the workspace calls it. | | `cli.apply_to_config` (`services/cli_params.py`) | Every listed type became `r.fail` with message only | Only a `CliParamsConfig` `ValidationError` becomes `r.fail(…, exception=exc)`. `params_apply` failures remain owner `r.fail`s. Everything else escapes. | Constants removed because nothing uses them now: `CLI_SAFE_EXCEPTIONS`, `ERR_SETTINGS_VALIDATION_FAILED`, `OUTPUT_TABLE_FORMATTING_OPERATION`. ### Consumers (grep of every member in the workspace) - `process_mapping`, `MappingProcessor` and `CLI_SAFE_EXCEPTIONS` have no consumers outside flext-cli. - `cmd_validate_settings`, `tables_resolve_config`, `tables_render` and `apply_to_config` are called only inside flext-cli (`services/cmd.py`, `services/tables.py`, `_utilities/formatters.py`, `services/_cli_parts/flextclicli_part_02.py`). - Other repos match only unrelated homonyms (`u.Web.validate_settings`, flext-core `_validate_settings`) or generated architecture docs. ## 2. Timing-sensitive runtime-process tests - `test_completion_before_wake_clear_returns_promptly` is now `…_ends_monitoring`. - The trace holds the monitor at `wake.clear()` until the waiter has set `process_done` and `wake`. It waits without a timeout; the old 2s cap is gone. - A monitor that lost that wake-up would sleep until its deadline and report `timed_out`. So `not outcome.timed_out` is the proof. The `elapsed < 2` assertion is removed. - `test_normal_root_exit_leaves_no_descendant`: - The descendant writes its pid and process group to a pipe, then blocks on an `Event`. - The root does a blocking `readline`, records it, and exits. - The proof is that the pid and process group no longer exist when `run_to_file` returns. - Removed: file polling, the heartbeat, `sleep(0.15)`, and the `< 5s` assertion. - Interpreters start with `-I -S`. - No timeout was raised. ## Local gates (worktree `fix/cli-fail-loud-remaining`) - `make gen` twice: exit 0 both times, and the diff hash is the same after each run (fixed point). - `make check`: exit 0 (`Total: 1 Success: 1`). - `make test-full`: - Run 1: exit 2, `1 failed, 1192 passed`, at load average ~25. The failure was `test_file_tools_yaml.py::test_single_source_returns_requested_model`, blocked in `os.fsync` for 36s during `atomic_write_text_file` (host I/O stall). It is not a test changed here. Both rewritten tests passed in that run. - Runs 2 and 3: exit 0 each, `1193 passed`, at load average ~20. Both rewritten tests passed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Follow-up to #202: makes the remaining exception-handling sites fail loud and rewrites two runtime-process tests so they assert behavior, not the host clock. Exceptions these call paths used to normalize into generic failures now escape with their cause; only the Pydantic `ValidationError`s the callers declare as outcomes are still reported as failures, and callers that relied on the old normalized messages now see the underlying exception. **Fail loud at remaining catch sites** - `cmd_validate_settings` and `tables_render` no longer catch anything; renderer errors now propagate instead of becoming `r.fail` messages. - `tables_resolve_config` and `apply_to_config` only report the model's `ValidationError` as a failure and now attach it via `exception=exc`; `params_apply` failures remain the `r.fail` results their owners produce. - Removes `process_mapping` and `MappingProcessor`; their skip mode swallowed exceptions into a debug log and collect mode aggregated them, and nothing in the workspace calls them. - Deletes the now-unused `CLI_SAFE_EXCEPTIONS`, `ERR_SETTINGS_VALIDATION_FAILED`, and `OUTPUT_TABLE_FORMATTING_OPERATION` constants. **Timing-free runtime-process tests** - The completion test holds the monitor at its first `wake.clear()` until the root waiter has set `process_done` and `wake`, so a lost notification would surface as `timed_out`; the `elapsed < 2` assertion, the child `sleep`, and the 2s wait caps are gone. - The descendant test has the child announce its pid and process group over a pipe and then block forever; the root reads the line and exits, and the test asserts the pid and process group no longer exist when `run_to_file` returns. File polling, the heartbeat, `sleep(0.15)`, and the `< 5s` assertion are removed. - Both scenarios start interpreters with `-I -S`. <sup>Written for commit 8280536. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/flext-sh/flext-cli/pull/203?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img](https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a){kind=link}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #201. It resolves three rule violations (DRY/SSOT, zero residue, fail loud).
1. One model-command owner
u.Cli.build_model_commandtogether with itsBuilderandSignatureCarrier. This builder duplicatedcli.model_commandand passed raw field annotations to Typer, so mapping and nested-model fields failed.cli.model_commandstays the single owner and routes structured fields throughu.Cli.is_json_option.u.Cli.derive_model, which copiedcli.derive_model.u.Cli.model_source_datastays because it is the primitivecli.derive_modeluses.build_model_command,u.Cli.derive_modelandFlextCliUtilitiesModelCommandsand found only the projected docs note that saysFlextCliCli.build_model_command(...)does not exist. That note is still true.2.
u.Cli.field_defaultno longer drops defaults silentlyexcept c.EXC_VALIDATION_TYPE_VALUE: Nonecatch and thecase _: Nonefall-through.Nonestill means "no default".cli.model_commandbuild time. The default-sourceValidationErrorescapes unchanged. A default that validates but that no Typer option can carry raisesTypeErrorwith the field name (c.Cli.ERR_FIELD_DEFAULT_NOT_CLI_VALUE_FMT). Structured defaults still go through the JSON-option path.3. Prompt failures propagate
CLI_SAFE_EXCEPTIONScatch fromconfirmand the_guarded/_fatalhelpers thatprompt,prompt_choiceandprompt_passwordused. An exception from the input or password reader now escapes with its cause.KeyboardInterruptandEOFErrorare still declaredr.failoutcomes ofconfirm, and each result now carries the exception.prompt_choicenever read input. Itsmessageparameter was used only by the removed failure log, so the parameter is gone. No member callsprompt_choice.CLI_SAFE_EXCEPTIONSitself stays becausecmd,tables,validationandcli_paramsstill use it.Tests
All tests go through the public service. Prompt tests use scripted input ports and check that reader failures propagate and that cancellation keeps its cause. Default tests build the command through
cli.model_commandand check that it fails with the cause. Tests that only asserted the old normalization were removed.Local gates (worktree
fix/cli-single-command-owner-fail-loud)make gentwice: exit 0 both times, no diff after the second run.make fmt: exit 0.make check: exit 0 (Total: 1 Success: 1).make test-full:test_completion_before_wake_clear_returns_promptlyfailed (3.09s, limit < 2s) andtest_normal_root_exit_leaves_no_descendanthit its 10s timeout. Both are timing-sensitive runtime-process tests and ran at load average ~9.8.🤖 Generated with Claude Code
Summary by cubic
cli.model_commandis now the single owner for model-backed commands, unrepresentable field defaults fail at command build time, and prompt input failures propagate with their cause.u.Cli.build_model_commandbuilder andu.Cli.derive_model;cli.model_commandandcli.derive_modelare the canonical entry points.u.Cli.field_defaultno longer swallows invalid defaults; a validated default no Typer option can carry raisesTypeErrornaming the field, and invalid default sources escape unchanged.KeyboardInterruptandEOFErrorremain declared outcomes and each result now carries the exception.prompt_choicenever read input, so itsmessageparameter is removed.Written for commit 0999f31. Summary will update on new commits.