You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
 
 
 
 
 
 

14 KiB

G-046 punk::args deferred -help resolution (parse-time performance + reentrancy) and rendering/value-shape fixes

Status: achieved 2026-07-10 Scope: src/modules/punk/args-999999.0a1.0.tm (resolve/get_dict: display-field deferral, dynamic-cache subst path, prefix writeback, string renderer, cmdhelp-facing messages), src/modules/punk/ansi-999999.0a1.0.tm (mark_columns argdoc as the reentrancy/perf testbed), src/tests/modules/punk/args/testsuites/ (GAP tests flip; perf verification) Acceptance: parsing/argument resolution provably skips -help expansion (a definition whose -help contains a ${[...]} that would error or record its invocation shows the substitution did NOT run during a parse-only path, only for help display); first parse of punk::ansi::mark_columns drops from ~4s to well under a second with 'i punk::ansi::mark_columns' still rendering the embedded example, and a -help that parses its OWN definition id resolves or errors cleanly rather than looping; first-parse timing improves for at least one other heavily documented command (recorded in the detail file); rendering_atdynamic_multiline_help_insertion_GAP flips to all-aligned; choicegroups_imap_prefix_listwrap_GAP flips to shape-identical (prefix input yields the same plain string as exact input); the -return string renderer's cmd-help continuations align under the first line with relative indents preserved (rendering_string_renderer_characterization updated); the 'Bad number of leading values...' prefix shown by goodargs parsing in 'i string is'-style output is reworded or suppressed for the usage-display path; full punk::args and punk::ns suites pass with no non-GAP expectations weakened.

Context

Drafted 2026-07-09 from defects found (and pinned as GAP tests) during the tests-first characterization pass. User emphasis at approval: "Any performance gain we can gain from deferring -help processing out of argument resolution would be worthwhile" - deferral is the headline item, not merely a reentrancy patch.

1. Deferred -help resolution (performance + reentrancy)

punk::args::resolve currently tstr-processes entire definition blocks, including -help sections. The in-source review note (resolve, near the block-processing loop) already observes: "-help section processing is sometimes expensive ... isn't required for parsing of arguments ... ideally we would delay expansion of -help sections until needed for display, and use a different cache key for the parsing vs display versions of the resolved definition."

The reentrancy class this also fixes, reproduced 2026-07-09 with punk::ansi::mark_columns: its argdoc -help contains ${[punk::ansi::argdoc::num_columns_example]}, and that example proc itself calls mark_columns, whose punk::args::parse resolves the same still-resolving definition. First call measured at ~3.94 SECONDS (60us thereafter, once cached). The mark_columns source (punk/ansi ~6443-6470) documents the hazard - including the observed "too many nested evaluations (infinite loop?)" when [a red] etc. are used in that -help - and lists the candidate fixes: (a) avoid parsing functions in argdocs (limiting, not DRY), (b) reentrancy guard, (c) don't process -help during argument resolution. This goal takes (c) as primary (with the perf benefit), with a reentrancy guard as defence-in-depth for the display path (a -help that renders help for its OWN id must error cleanly or resolve, never loop).

Design notes:

  • separate cache keys for parse-spec vs display-spec (per the in-source note); the @dynamic unresolved-cache (ptlist/paramlist subst mechanism) splits accordingly.
  • fields in display-only scope: -help on @cmd/@examples/args, -choicelabels content, @formdisplay bodies - anything not consulted by get_dict for matching/validation. (-choices/-choicealiases/-typesynopsis ARE parse-relevant or synopsis-relevant - classify explicitly in implementation.)

2. @dynamic multiline paramindent gap

rendering_atdynamic_multiline_help_insertion_GAP: a ${$DYN_X} double substitution whose second-round result is multiline lands lines 2+ at insertion-column minus 4 (internal relative indents preserved), because the cached-path plain subst has no paramindent re-alignment - contrast single-level ${[cmd]} insertion which aligns fully (rendering_tstr_multiline_command_insertion). Fix: apply the same paramindent treatment to second-round substitution results. Note interaction with item 1: if display fields defer, the fix belongs in the display-time expansion path.

3. Prefix-normalization value shape (list-wrap)

choicegroups_imap_prefix_listwrap_GAP: for choices containing backslashes (punk::imap4's {\Deleted} etc. - spaces would behave likewise), exact input stores the plain string but prefix-normalized input stores a one-element LIST ({\Deleted}) - the get_dict overwrite writeback lsets into the stored value treating a scalar as a list. Consumers get a different value shape depending on how the user typed the choice. Fix the writeback for the single-value case (the -multiple/clause paths already index deliberately).

4. -return string renderer cmd-help alignment

rendering_string_renderer_characterization: the string renderer joins the -help first line onto the "Description: " label but drops continuations flush-left (relative indents preserved among themselves). The table renderer aligns properly. Align continuations under the first line. (The characterization test's pinned flush-left expectations get updated to the aligned form.)

5. cmdhelp goodargs message accuracy

'i string is' (and any command whose definition requires leaders) prefixes its usage output with "Bad number of leading values for punk::args::parse $args_remaining ..." - an internal-looking, inaccurate message produced by cmdhelp's goodargs-marking parse of the (empty) trailing args. Reword for the usage-display path or suppress (the parse is only advisory for highlighting supplied args). User: "the message is not accurate and could do with a fix up at some stage."

Implementation design (item 1, decided 2026-07-10)

Masking approach - the existing tstr pipeline stays authoritative for everything eager:

  1. Phase 0 (per block containing ${): tstr -return list -eval 0 extracts (textparts, params) without evaluation. Assign each param a globally numbered inert placeholder token and build the joined placeholder text.
  2. Phase 1 (classification): split the placeholder text into records (the record splitter factored out of resolve into a shared private proc) and classify each placeholder by the field it lands in. Display-only fields: -help on @cmd/@examples and on argument records, -choicelabels values, @formdisplay bodies. Everything else (including @cmd -summary, -choices, -choicealiases, -typesynopsis, @form -synopsis) stays eager so synopsis/parse never need the display spec.
  3. Phase 2 (masked rebuild): for blocks carrying display params, rebuild the block with display ${...} sources replaced by their placeholder tokens, then run the EXISTING tstr -return string -eval 1 -allowcommands -undent 1 call unchanged - eager params get bit-identical current behaviour (alignment, undent, errors); placeholders pass through as literal text into the parsed argdata (so -help values in the parse spec contain tokens, never expanded content).
  4. argdata gains a deferred-params map (token -> {param source, defspace, dynamic flag}). get_dict never consults display fields, so parsing works from this spec directly.
  5. Display expansion (new private helper): walks the known display slots of a spec dict, evaluates each token's param (namespace eval $defspace [list ::subst $param]; for @dynamic definitions a second subst round runs on the result - this is where the round-2 paramindent re-alignment (item 2) is implemented), inserts multiline results aligned to the leading whitespace of the token's line within the stored field value (mirroring tstr -paramindents line). Non-dynamic display specs are cached; @dynamic display expansion re-runs per render (preserving provider-refresh semantics).
  6. Hooks: get_spec returns the display-expanded spec (public consumers see today's shape); arg_error expands any spec dict still carrying a deferred map (parse-error -argspecs travel unexpanded). synopsis stays on eager fields only.
  7. Reentrancy guard (defence in depth): an in-progress set keyed by definition id; a display expansion re-entered for the same id substitutes the raw ${...} source instead of recursing, so a -help that renders help for its own id resolves cleanly.

@dynamic interaction: round-1 display params are masked (so their ${[provider]} payloads never run at parse time); the round-2 unresolved-cache (ptlist/paramlist of the round-1 OUTPUT) carries the tokens inertly in its text parts, while round-2 parse-relevant params keep refreshing every resolve exactly as today.

Verification (2026-07-10, punk::args 0.4.2 / punk::ns 0.1.4)

Perf (punk902z src, this machine; OLD numbers measured same-day with the pre-deferral module stashed back in):

  • punk::ansi::mark_columns first call: OLD 4317.6ms -> NEW 12.4ms (subsequent 0.5ms); first 'i'-equivalent help render 52ms with the embedded num_columns_example ruler rendering correctly and no deferral tokens in output.
  • tclcore ::lseq (helpers::example blocks): first parse-spec resolve OLD 184.4ms -> NEW 1.8ms; the display cost (581ms incl. table layout) is now paid only on first help render.

Behaviour (all in src/tests/modules/punk/args/testsuites/args/deferredhelp.test):

  • parse provably skips -help ${[...]} (invocation-recording probe: 0 calls across parses, 1 after first display, cached for non-dynamic thereafter).
  • -help substitution parsing against its OWN id resolves cleanly at display time (parse inside the helper uses the already-cached parse spec).
  • -help substitution rendering usage for its OWN id completes via the reentrancy guard (nested expansion substitutes raw ${...} sources).
  • @dynamic deferred -help re-expands per display (provider refresh preserved).

GAP flips: rendering_atdynamic_multiline_help_insertion (was ..._GAP) now all-aligned [0 0 2 0]; choicegroups_imap_prefix_shape (was ..._listwrap_GAP) shape-identical; rendering_string_renderer_characterization updated to aligned cmd-help continuations. cmdhelp goodargs message suppressed for the no-args usage-display path (punk/ns cmdhelp.test cmdhelp_leader_required_no_args_plain_usage; live 'i string is' verified clean).

Suites: punk/args 148 (147 pass/1 pre-existing skip) + punk/ns 49 pass on tcl903 and tcl87; full source-tree suite 707 tests with only the documented exec-14.3 baseline failure. No non-GAP expectations weakened (characterization updates assert the fixed behaviour per acceptance).

Post-achievement follow-up (2026-07-10, user-reported): 'i punk::args::define' timing roughly doubled (~5.2/4.0s -> ~9.9/8.1s). Cause was the first cut of the item-5 fix, not the deferral: for a definition with required args and no supplied words, the advisory parse (at -errorstyle standard/basic) rendered the full usage table inside the raised error, which the new plain-usage branch then discarded and re-rendered - two table builds per 'i'. Fixed by running the no-supplied-words advisory parse with -errorstyle minimal (cheap failure, single render). Like-for-like on punk91 src: pre-change 5299/4083ms, post-fix 5360/4176ms (parity within noise; residual cost is table/ANSI construction, explicitly out of G-046 scope). The define argdoc's single deferred ${[helpers::example ...]} expands once into the display cache as designed.

Alternatives considered

  • Reentrancy guard alone (without deferral) for item 1 - rejected as primary: it fixes the loop but forfeits the first-parse performance win the user explicitly wants, and heavy -help processing would still run on every first parse.
  • Documenting "don't call parsing functions from argdocs" - rejected (the mark_columns source itself calls this limiting and non-DRY).

Notes

  • GAP tests to flip: rendering_atdynamic_multiline_help_insertion_GAP, choicegroups_imap_prefix_listwrap_GAP; characterization tests to update: rendering_string_renderer_characterization. Safety net: the full 128-test punk/args suite (esp. rendering.test invariants and dev/dynamic-cache.test) and punk/ns cmdflow.
  • Perf evidence baseline (2026-07-09): mark_columns first parse ~3.94s / subsequent 60us (punk902z dev, this machine). tclcore moduledoc commands (e.g. ::lseq with helpers::example blocks) are candidates for the second timing datapoint.
  • Pre-existing adjacent defect found 2026-07-10 (assessed, not in this goal's scope; candidate goal flagged): 'i ' from a namespace that hasn't loaded the ensemble's argdocs renders the autogenerated ensemble help without subcommand-help markers/choicelabels. Root cause: punk::ns::arginfo (cmd_traverse entry, ns-999999.0a1.0.tm ~4886) calls punk::args::update_definitions with [namespace qualifiers $origin] (e.g. ::punk::ansi for ensemble ::punk::ansi::ansistring), but the subcommand argdocs live in the separately registered pkgns ::punk::ansi::ansistring (its ::argdoc child), which never gets loaded on that path; punk::args::ensemble_subcommands_definition (and the older inline path in punk::ns::arginfo ~4610) then run their punk::args::id_exists checks against unloaded ids. Self-heals once anything loads the ns (autodef regenerates per 'i' call). No existing test pins the ordering. G-046 relevance: (a) 'i ' output/timing used during G-046 verification depends on session load history - pin update_definitions state explicitly in any such test/timing; (b) strengthens the item-1 classification note: autodef generation computes choicelabels eagerly via punk::args::synopsis -return summary per subcommand, so @cmd -summary and synopsis inputs must resolve from the cheap parse-spec path, never force full -help expansion per subcommand.
  • Related: G-045 (authoring ergonomics - same resolve neighbourhood; free sequencing), G-041 (form matching will exercise resolve paths more heavily - deferral lightens its per-form attempts), G-044 (completion wants fast parse-spec resolution).