feat(tui): tirith ui — explore results, build policies, experiment in a playground - #280
feat(tui): tirith ui — explore results, build policies, experiment in a playground#280AkashS0510 wants to merge 9 commits into
tirith ui — explore results, build policies, experiment in a playground#280Conversation
…in a playground Add an interactive interface with three tabs, as an optional extra. Explorer. The result document has always carried each result's resource address, its planned action and its before/after values, but the pretty printer prints only the message -- so on a wildcard policy over a real plan you get hundreds of lines reading `"product-456"` is not empty with nothing to say which resource each came from, and the detail is reachable only by piping --json into another tool. This shows it: the tree opens on the first failure, names replacements by their ordering (destroy-first and create-first mean different things), and lists the attributes that changed, flagging the ones that are unknown until apply. Builder. A form whose fields follow the chosen provider and operation, because the argument names are not guessable and are documented nowhere machine-readable -- stackguardian/json reads key_path while stackguardian/kubernetes reads attribute_path, and terraform_plan alone has seven operations taking different arguments. Get one wrong and the policy still parses, still runs, and silently matches nothing. Values keep their JSON types, so `Equals: true` and `Equals: "true"` stay distinguishable. The eval_expression is an editable field: it fills itself in with every check &&-ed together and stops the moment you edit it, since an expression is the one part of a policy that cannot be derived from the checks. Playground. Edit a policy and an input side by side and watch the verdict move, debounced. Five worked examples, most of which fail on purpose, each with notes on the mechanism it demonstrates. Broken JSON, half-written policies and providers that raise are reported in a findings pane rather than as tracebacks -- while editing, the broken state is the normal state. A file browser modal opens documents from anywhere on disk. Supporting pieces, all free of any UI-toolkit import so they run on the 3.8 CI leg: - tui/schema.py, a hand-written provider table, because the engine has nothing to introspect. tests/tui/test_schema_matches_providers.py asserts every provider, operation and argument named in it appears in the real handler, so drift fails CI rather than leaving the builder able to generate a policy the engine cannot run. It caught one error while being written. - tui/validate.py, reporting what the engine leaves silent: an argument the operation does not read, an id referenced but never defined, a single `&` where `&&` was meant. Advisory, not blocking. Every policy fixture in the repository that the engine can actually run validates clean; the three it cannot are listed with the reason. - tui/results.py, a read-only view over the result document, so the Explorer can also open a --json file captured from a CI run months ago. Optional: `pip install 'py-tirith[tui]'`. Not a hard dependency -- the interface needs Python 3.9 while tirith supports 3.8, and using tirith as a CI gate should stay dependency-light. Without it, `tirith ui` prints how to install it and exits 1. The local evaluation surface is untouched. `ui` is dispatched before the flat parser, exactly as `platform` is, so --json output remains byte-identical to the golden file.
Three violations the linter caught, all in the new module: - D105 on Finding.__str__, which had no docstring. - D301 on two docstrings containing backslashes -- the regex \b in _names_in_expression, and the escaped quotes in the Explorer's module docstring. Both are now raw strings, which also lets the Explorer's example read as `"product-456"` rather than through a layer of escaping.
textual-serve prints its own three-row TEXTUAL-SERVE wordmark and the full `python -m tirith ui` command line at startup, which tells the reader about our implementation rather than about the thing they just started. Override on_startup -- the supported extension point, documented as such on the base class -- with a TIRITH wordmark drawn in the same style, the version, and the URL. Every row of the wordmark is 21 characters. The first attempt had the second T one column short, at 21/22/22, so its stem ran into the H beside it. A test compares the row widths, since that is the property that was wrong and is not something anyone notices by rereading the string.
…tab row Two problems, both from the wordmark occupying a band of its own. The banner, tab row, pane headers and footer are all fixed height, and on a shorter terminal they summed past the viewport -- so the Screen scrolled. That put a scrollbar down the far right edge of the *application*, outside any pane, and let the wordmark scroll out of view, leaving a bare tab row at the top. Screen is now overflow: hidden, since each pane scrolls its own content and the app as a whole never should. The wordmark moved onto the tab row on the overlay layer, costing no height at all. Docked right and sized to its text: a full-width overlay swallows the row it spans whether or not it paints there, which hid the tab labels underneath it. Tests cover both -- that nothing but the two editors scrolls at a short size, and that the wordmark sits clear of the labels.
…cannot run Two failures that together made "Open in playground" look broken. Add check accepted a check whose operation was missing required arguments, so pressing it on an empty form appended a check with no provider_args at all -- three presses gave three of them. It now refuses, naming the fields to fill. Open in playground then handed that policy over anyway, dropping the user into the Playground showing "Policy is incomplete" with no results. That reads as the button failing rather than the policy being unfinished, and leaves them in the wrong tab to fix it. It now validates first and stays on the Builder, where the findings pane already says what is wrong. test_an_unset_optional_select_is_left_out_of_the_policy moved from provider_config to attribute: provider_config's own `attribute` argument is required, so under the new rule that check is correctly refused, and the test was no longer exercising an optional argument at all.
…he panes Move the wordmark to the left of the tab row, so the name is read before the tabs rather than trailing them, and indent the tabs clear of it. The indent is a CSS value that has to track the banner string and gets it wrong quietly: at 27 against a 32-column banner, "Explorer" rendered as "orer". A test now compares the two measured regions, which catches that where reading either alone would not. Give the Playground's header row the same left margin as the toolbar labels beneath it, so "Example:" starts on the same column as "Policy" and "Input document". Previously the label began hard against column 0 while the description ran to the far edge, and nothing in the row lined up with anything below it.
…he validator codecov/patch failed on the new module, but the tests were not the problem: the coverage job installs with `pip install .`, without the 'tui' extra. The interface's tests then skip -- correctly, since the toolkit is absent -- and every line behind them counts as uncovered. app.py, render.py and all four views reported 0%, roughly 740 lines the existing tests do exercise. That is a coverage report describing the environment rather than the tests, so the job now installs '.[tui]'. With that fixed the weakest module was validate.py at 78%. Its untested lines were the malformed-policy branches -- meta that is not an object, a missing eval_expression, a value outside a closed choice list, a single `|` -- which is the validator's whole reason for existing, so they are worth pinning rather than merely counting. Now 95%. Also covers tui/cli.py's file-reading and missing-extra paths. What is left there needs a real terminal (the /dev/tty reattach, app.run) or a live server (--serve); those were verified by hand instead. Overall 84%, with every tui module between 86% and 95%.
There was a problem hiding this comment.
Code review of the tirith ui branch — 7 findings, each reproduced against this branch (textual 8.2.8). Details inline. The headline one is (1): check_policy raises TypeError out of the Playground for a policy whose required_provider or condition.type is a list/object, which kills the app — exactly the traceback-while-editing case the module docstring says it exists to prevent.
Reviewed by Claude claude-opus-5
| ) | ||
| ) | ||
| return "" | ||
| if provider_name not in PROVIDERS_DICT: |
There was a problem hiding this comment.
check_policy raises TypeError on an unhashable JSON value, crashing the Playground.
provider_name is whatever the buffer parses to. When it is a list or an object, provider_name not in PROVIDERS_DICT raises TypeError: unhashable type: 'list'. Same fault at line 229 (evaluator_name not in EVALUATORS_DICT) and line 248 (schema.EVALUATORS.get(evaluator_name or "")).
check_policy is called outside any try in playground.evaluate_now (line 312) and in builder._refresh_preview (line 411), so the exception escapes the Textual handler and takes the app down with a traceback — the exact case this module's docstring says it exists to prevent ("an exception is the normal case and a traceback is not an acceptable way to render it").
Reproduced on this branch: type "required_provider": ["stackguardian/json"] (or "type": ["Equals"]) into the policy editor and the app dies at validate.py:96 with TypeError: cannot use 'list' as a dict key. A plausible mistake — the neighbouring resource_type argument really is a list.
Guard the lookups with isinstance(..., str) before using the value as a dict key, and report a _error(...) for the non-string case.
There was a problem hiding this comment.
Confirmed and fixed in d6cedc2. Both lookups now check isinstance(str) before the membership test and report the non-string as a finding; condition.type is normalised to None so the third site (schema.EVALUATORS.get) is safe too. Regression tests cover list and dict at both keys.
| asked to look at with an unrelated one. The first playground run after that is the | ||
| user's own doing, so it takes over from then on. | ||
| """ | ||
| if self._explorer_is_pinned: |
There was a problem hiding this comment.
--result combined with --input throws away the result the user asked to explore.
The pin is consumed by the first adopt, but two evaluations happen during startup:
PlaygroundView.on_mountloads bundled example 01 and evaluates it →_adopt_reportSetUp basic CLI support for Module 2 repo #1 → unpins and returns.TirithApp.on_mountcallsplayground.load_documents(None, self._initial_input), which evaluates again →_adopt_reportInitialise Module 2 repo #2 → overwrites the Explorer with example 01's policy run against the user's input.
tirith ui --result r.json --input plan.json is accepted by the CLI (only --result + --policy is rejected), so the user gets an unrelated evaluation in the Explorer. Verified with build_app(report=<parsed r.json>, input_document={...}): the Explorer ends up holding costcenter_tag_present instead of the checks from r.json.
Either keep _explorer_is_pinned set until a user-initiated run (a real edit / the Run button), or reject --result alongside --input the way --result + --policy is rejected.
There was a problem hiding this comment.
Reproduced — the Explorer held example 01 instead of the --result. Fixed in d6cedc2 by releasing the pin on a deliberate action (edit, Run, opening a document, choosing an example) rather than counting down the first adopt. Two tests: the result survives startup with --input alongside it, and a user Run still hands the Explorer over.
| # dependency is install time on every run -- people gating a pipeline should not pay | ||
| # for an interface they never open. | ||
| "tui": [ | ||
| 'textual>=0.60; python_version >= "3.9"', |
There was a problem hiding this comment.
The declared textual floor is far below what the code actually requires.
Select.NULL — relied on in playground.on_select_changed and builder._collect_provider_args — only exists in textual >= 8.0.0. In every release from 0.60 through 7.x the sentinel is named Select.BLANK and NULL does not exist (checked the wheels for 0.60.1, 0.86.0, 1.0.0, 2.1.2, 3.5.0, 5.0.1, 6.1.0, 7.0.0). Select(compact=True) in playground.compose needs >= 3.x as well; on older textual it is a TypeError at compose time, so the Playground tab never builds.
A fresh pip install 'py-tirith[tui]' happens to resolve textual 8.x today, but anything that caps textual — another tool in the venv, a constraints file, a lockfile — resolves to a version this metadata declares as supported and then fails at runtime with AttributeError: type object 'Select' has no attribute 'NULL' the first time the prompt row of the example picker is selected.
| 'textual>=0.60; python_version >= "3.9"', | |
| 'textual>=8.0; python_version >= "3.9"', |
There was a problem hiding this comment.
Right, Select.NULL is 8.0+ (Select.BLANK before that). Raised to textual>=8.0 in d6cedc2.
| "count", | ||
| "How many resources of a type the plan changes.", | ||
| [_RESOURCE_TYPE], | ||
| ), |
There was a problem hiding this comment.
exclude_resource_types is missing from count and action, so the validator emits a false "will be ignored" warning.
terraform_plan/handler.py reads exclude_resource_types once at the top of provide() (line 65) and honours it in three branches: attribute (line 87), action (line 158) and count (line 187). The table attaches it to attribute only.
Two consequences: the Builder cannot generate it for count/action, and — worse — a correct existing policy is reported as wrong. Reproduced:
>>> check_policy({... 'operation_type': 'count', 'terraform_resource_type': '*',
... 'exclude_resource_types': ['aws_iam_policy'] ...})
["evaluators[0].provider_args.exclude_resource_types: Not read by operation 'count'; it will be ignored."]
The argument is read and does exclude the type. test_schema_matches_providers.py cannot catch this: it only asserts table → handler, never handler → table.
Add the shared Arg to the count and action operations.
There was a problem hiding this comment.
Good catch — the handler reads it once and honours it in all three branches. Added to count and action in d6cedc2. Also added the reverse-direction check you identified as missing: a test that counts the branches consulting it in the handler and requires the table to declare as many.
| lines.append("[bold]Failing resources[/bold]") | ||
| for result in failing: | ||
| name = result.resource.label or render.escape(result.message) | ||
| lines.append(f" ✘ {render.escape(name)}") |
There was a problem hiding this comment.
Double escaping — the message fallback gets a literal backslash on screen.
Line 145 already escapes the fallback (or render.escape(result.message)), and line 146 escapes name again. Rich renders the result with a stray backslash:
>>> Text.from_markup(escape(escape('bucket[0] is not empty'))).plain
'bucket\\[0] is not empty'
This is the non-terraform path (json/sg_workflow results carry no resource.label), and terraform addresses with an index are exactly the case escape was written for. Drop the escape on line 145 and let line 146 do it once.
There was a problem hiding this comment.
Fixed in d6cedc2 — dropped the escape on the fallback. Worth noting the test asserts on the rendered text rather than the markup; my first attempt checked the markup, where one backslash is escaping working correctly, and would have failed on a correct implementation.
|
|
||
|
|
||
| def _quote(part): | ||
| return f'"{part}"' if " " in part else part |
There was a problem hiding this comment.
_quote is not shell quoting, and textual-serve runs the command through a shell.
textual_serve/app_service.py launches with asyncio.create_subprocess_shell(command), so this string is interpreted by sh. Double quotes do not protect $, a backtick, a \ or an embedded ".
tirith ui --serve --policy '/tmp/my $work/policy.json' → the shell expands $work to empty → every browser session runs tirith ui --policy /tmp/my /policy.json, prints ERROR: --policy ... does not exist and exits, with nothing in the serving terminal to explain why.
shlex.quote is the drop-in fix (with import shlex alongside the other imports at the top):
| return f'"{part}"' if " " in part else part | |
| return shlex.quote(part) |
There was a problem hiding this comment.
Confirmed create_subprocess_shell. Switched to shlex.quote in d6cedc2, with a test that round-trips a $-containing path back through shlex.split.
|
|
||
| try: | ||
| from .app import build_app | ||
| except ImportError: |
There was a problem hiding this comment.
A blanket except ImportError here also swallows real import errors inside the TUI package.
from .app import build_app pulls in app.py, three view modules, filepicker, schema, validate and core.core. Any ImportError raised anywhere down that chain — a typo'd symbol in views/playground.py, a textual API removed in a later release, a broken relative import — is reported as "install the 'tui' extra", telling a user who already has it installed to install it again, with the traceback discarded.
Narrow it to the missing toolkit, e.g. except ImportError as e: plus a check that the failing module is textual, and re-raise otherwise.
There was a problem hiding this comment.
Fixed in d6cedc2 — narrowed to ImportError.name naming textual or textual_serve, anything else re-raises. Sidenote: my first regression test simulated the missing extra by evicting tirith.tui.* from sys.modules, which hung the suite when a later test re-imported the package under a broken textual. It patches a single module object now.
All seven reproduced on this branch before fixing, and each has a regression test. 1. check_policy raised TypeError on an unhashable JSON value. A list or object at meta.required_provider or condition.type made `x not in PROVIDERS_DICT` raise rather than return False, and check_policy is called outside any try in the Playground, so the exception escaped and killed the app mid-edit -- the exact failure this module exists to prevent. Both lookups now check isinstance(str) first and report the non-string as a finding. 2. --result was thrown away when --input was given alongside it. Two evaluations run while mounting -- the Playground's own example, then the supplied input -- so a pin consumed by the first was defeated by the second, and the Explorer ended up holding example 01 evaluated against the user's plan. The pin is now released by a deliberate action (an edit, Run, opening a document, choosing an example) rather than counted down. 3. The declared textual floor was >=0.60, but Select.NULL only exists from 8.0 -- before that the sentinel is Select.BLANK. Anything that caps textual resolved to a version this metadata called supported and then raised AttributeError the first time the example picker's prompt row was chosen. Raised to >=8.0. 4. exclude_resource_types was described against `attribute` only, while the handler reads it once and honours it in the attribute, count and action branches alike -- so the validator told the author of a correct count policy that the argument would be ignored. Added to all three, with a test in the reverse direction (handler -> table), which is the direction the existing drift-guard never checked. 5. The Explorer escaped its message fallback twice, putting a literal backslash on screen for every result without a resource label -- which is every json and sg_workflow result. 6. _quote wrapped in double quotes, but textual-serve launches with create_subprocess_shell, so `$`, backticks and backslashes stayed live: a path like `/tmp/my $work/policy.json` had $work expanded away and every browser session started against a path that did not exist. Now shlex.quote. 7. A blanket `except ImportError` around `from .app import build_app` -- which pulls in four views, the schema, the validator and the engine -- told someone whose toolkit was installed to install it again and discarded the traceback for the real fault. Narrowed to ImportError.name naming textual or textual_serve; anything else re-raises.
|
❌ The last analysis has failed. |
2 participants
Adds an interactive interface with three tabs, as an optional extra. The local evaluation surface is untouched.
Why
The result document has always carried each result's resource address, its planned action and its before/after values. The pretty printer prints only the message — so on a wildcard policy over a real plan you get hundreds of lines reading
`"product-456"` is not emptywith nothing to say which resource each came from, and the detail is reachable only by piping--jsoninto another tool.The three tabs
Explorer — opens on the first failure, names the resource (
aws_db_instance.primary), gives the action in terraform's vocabulary (replace (destroy first) — distinct from create-first, because only one of them means downtime), and lists the attributes that changed, flagging the ones unknown until apply.Builder — a form whose fields follow the chosen provider and operation. The argument names are not guessable and are documented nowhere machine-readable:
stackguardian/jsonreadskey_pathwhilestackguardian/kubernetesreadsattribute_path, andterraform_planalone has seven operations taking different arguments. Get one wrong and the policy still parses, still runs, and silently matches nothing. Values keep their JSON types, soEquals: trueandEquals: "true"stay distinguishable.eval_expressionis editable (&&,||,!, grouping); it auto-fills and stops the moment you edit it, since it is the one part of a policy that cannot be derived from the checks.Playground — edit a policy and an input side by side and watch the verdict move. Five worked examples, most of which fail on purpose, each with notes on the mechanism it demonstrates and what to try next. Broken JSON, half-written policies and providers that raise are reported in a findings pane rather than as tracebacks — while editing, the broken state is the normal state.
Supporting pieces
All free of any UI-toolkit import, so they run on the 3.8 CI leg:
tui/schema.py— a hand-written provider table, because the engine has nothing to introspect (terraform_plandispatches through an if/elif chain;sg_workflowhas nooperation_typeat all).tests/tui/test_schema_matches_providers.pyasserts every provider, operation and argument named in it appears in the real handler, so drift fails CI rather than leaving the builder able to generate a policy the engine cannot run. It caught one error while the table was being written.tui/validate.py— reports what the engine leaves silent: an argument the operation does not read, an id referenced but never defined, a single&where&&was meant. Advisory, not blocking. Every policy fixture in the repository that the engine can actually run validates clean; the three it cannot are listed with the reason.tui/results.py— a read-only view over the result document, so the Explorer can open a--jsonfile captured from a CI run months ago.Compatibility
uiis dispatched before the flat parser, exactly asplatformis, so--jsonoutput stays byte-identical to the golden file.tirith uiprints how to install it and exits 1 — verified from a clean wheel install.Verification
black --checkclean across 108 files--serveconfirmed serving over HTTP and rendering in a browserOne pre-existing failure on
mainis unaffected:test_e2e_inline_varsshells out to atirithbinary that is not on PATH in a source checkout, and fails identically with these changes stashed.