Render In[T] and graph_input defaults as the schema's string (v0.1.25) - #68
Merged
Volv-G merged 1 commit intoSep 25, 2026
Conversation
`InputSpec.default` is `string | null`, but the tracer wrote a signature default straight through, so `threshold: In[int] = 5` — the shape `In`'s own docstring shows — emitted `default: 5`. Nothing rejected it: the local validator passes it, and authors worked around it by declaring numbers as `In[str] = "60"`. Defaults now go through one serializer: strings unchanged, numbers and booleans rendered (`True`, matching @task component inputs and the corpus), lists and dicts as JSON with sorted keys, `None` still written as `default: null`. A default whose type contradicts the declared `T` is refused, as is a type with no sensible rendering and a non-finite float; diagnostics name the input and the type, never the value. `graph_input(default=...)` accepts the same values instead of demanding a pre-rendered string, and keeps passing one through untouched so ported YAML emits the bytes it always did. All 592 defaults in the pipeline corpus round-trip unchanged. The @task component path keeps its lenient `str(value)` fallback: it is a long-standing public surface whose users we cannot survey, and the bug is not there.
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
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.
(AI-assisted)
What
InputSpec.defaultisstring | nullin the pipeline schema, but the tracer wrote a signature default straight through. Sothreshold: In[int] = 5— the shapeIn's own docstring shows — emitted:Nothing caught it.
collect_pipeline_spec_errorsreturns[]for that document, so the only symptom was downstream. Authors had already worked around it by lying about the type:sample_size: In[str] = "0",timeout_secs: In[str] = "60",retry_count: In[str] = "4"all appear in our monorepo. After this change they can declare the type they mean.Defaults now go through one serializer, on both the signature route and
graph_input(default=...):"x"x5,-1'5','-1'1.5'1.5'In[float] = 1emits'1', not'1.0'True/False'True'/'False'{"b": 1, "a": 2},[2, 1]'{"a": 2, "b": 1}','[2, 1]'Nonenulldatetime,Path, …nan,infBoolean spelling follows the two things that already exist rather than a preference:
@taskcomponent inputs serializeTrueas"True", and the corpus writes Python spelling 49 times against 13. The generated container's_deserialize_boollowercases before matching, so both parse — a test pins that pairing instead of assuming it.A default's type must agree with the declared one:
In[int] = 1.5,In[str] = 5andIn[int] = Trueare all refused.boolis a Pythonintsubclass but not a TangleInteger, so that last one needs an explicit guard.In[float] = 1is allowed and renders'1', because whole-number Float defaults are idiomatic and the corpus already contains'0'and'72'.How
New
tangle_cli/input_defaults.py— stdlib-only,error_clsinjected by the caller, the same shape aseditor_layoutfrom 0.1.22. A test spawns a subprocess to confirm importing it pulls in notangle_cli.python_pipelinemodule, so the minimal-install packaging guards stay green. NewInvalidInputDefaultError(CompileError)for the signature route.The
@taskcomponent path keeps its lenientstr(value)fallback. Tightening it looked free — 55strand 2Nonedefaults across every@taskin our monorepo, nothing exotic — but that is a long-standing public surface whose users we cannot survey, a newly-failing compile there would have no migration path, and the reported bug is not there. The two paths never disagree about a value they both accept; a test asserts the Boolean spelling matches across them. If we later want the component path strict, it is a one-line delegation.Failure modes
graph_input(default=...)changes behaviour relative to 0.1.23. A non-string default used to raiseInvalidGraphIoError; it is now rendered. That is strictly more permissive, so no previously-valid call breaks, and the one 0.1.23 test asserting the rejection is inverted here on purpose — it is the only pre-existing test whose meaning this PR changes.A pre-rendered string stays accepted whatever the declared type is, on that route only. Applied naively, the type rule would reject
graph_input("n_samples", "Integer", default="-1")— which is how all 592 defaults in the corpus are written and exactly what a ported_gincall produces. Agraph_inputtype is a Tangle type string describing the wire form, where every value is text;In[int]is a Python annotation, soIn[int] = "41"is a mislabelled signature and is still refused. This is the one place the two surfaces deliberately differ, and it is documented.Nothing in the wild changes. Every one of the 592 corpus defaults was replayed through the new serializer with its declared type: 0 changed, 0 rejected. No Python pipeline in the monorepo uses a non-string
In[T]default, so no compiled output moves.graph_input(default=None)still raises — omit the argument to declare no default. A signature= Nonestill emitsdefault: null, which is schema-valid; rewriting it would churn documents for no gain.Review focus
_check_declared_type, and whether the signature/graph_inputasymmetry is the right call.@taskcomponent path lenient.Tophatting
40 tests on real compile output, each rule exercised through both routes: acceptance per type, YAML-level quoting (
default: '41'in the file, not just in memory), byte-identity for string defaults, every refusal, and diagnostics checked against a planted secret and a distinctive number.Six mutations, each caught: dropping the type check, confusing
boolwithint, dropping the finite check, restoring thestr(value)catch-all, droppingsort_keys, and dropping the pre-rendered-string escape. The catch-all mutation initially survived — the type check masked it — so there is now aJson-typed case where no type rule applies and only the supported-types gate stands between aPathand text nobody meant to ship.Checklist
pyproject.toml,packages/tangle-cli/src/tangle_cli/__init__.py,tests/test_packaging.py, and theuv.lockeditable self-entry (lock diff is the one-line version change only).