Skip to content

Make payload projection selector errors atomic - #84

Merged
rickardvh merged 6 commits into
masterfrom
codex/payload-project-bounded-selector-errors
Jul 19, 2026
Merged

rickardvh merged 6 commits into
masterfrom
codex/payload-project-bounded-selector-errors

Conversation

@rickardvh

Copy link
Copy Markdown
Owner

Summary

  • make payload.project reject unknown selectors atomically instead of returning partial values
  • add a bounded selector-validation error envelope with unknown selectors, inventory count/sample, discovery commands, and suggestions
  • update generated Python/TypeScript runtime templates plus projection docs/tests

Validation

  • uv run pytest tests/test_primitive_executor.py -k payload_project -q
  • uv run ruff check src/command_generation/primitive_executor.py src/command_generation/targets/python.py src/command_generation/targets/typescript.py tests/test_primitive_executor.py tests/primitive_conformance.py
  • uv run python tests/primitive_conformance.py

Stacked dependency for Agentic Workspace #2259.

@rickardvh rickardvh left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking review on d4134ca:

  1. The error path still constructs the selector catalog. _selector_validation_error() calls _available_selectors_for_payload(payload) and retains the resulting list before sampling it. That violates the linked issue's requirement that the full inventory never be materialized on the ordinary error route. Because list traversal is capped at ten entries per list, available_count is also not the true total for larger payloads. The response remains unbounded by request size through requested_selectors, unknown_selectors, suggestion keys, and selector token lengths.

  2. Validation still follows projection work. Valid selectors are resolved and stored before a later invalid selector is detected. In the generated Python runtime, _field_by_path() deep-copies selected values, so a large valid subtree can be copied and discarded on atomic failure. Use a first validation pass that performs existence checks without copying or serializing, followed by projection only after all selectors validate.

  3. The generic primitive invents host CLI semantics. The docs assign inventory/detail commands to the host, but the implementation synthesizes --select ... --format json and --verbose --format json. Those flags are not guaranteed generic command surfaces. Exact discovery/detail commands should be host-declared data that the primitive passes through unchanged.

  4. The semver classification is too weak. This unconditionally replaces the documented selected-output shape (values, missing, available_selectors) with a different kind and field contract. That is not a patch-compatible change unless introduced through an opt-in/versioned contract.

  5. Generated-target conformance is incomplete. The changed tests cover the in-process executor, not the generated Python and TypeScript runtimes through one canonical contract fixture. There is already a parity difference: Python str.replace() replaces every matching segment, while TypeScript String.replace() replaces only the first. Add shared target conformance for atomic failure, hard bounds, host-provided commands, custom kinds, and accurate counts.

Required direction: implement a bounded/streaming inventory summary with exact count, fixed sample and suggestions; bound selector count and token length in the shared parser; prevalidate before projection; move command text into host declarations; and prove identical behavior through generated Python and TypeScript targets.

@rickardvh rickardvh added semver:minor Release as a minor semver bump and removed semver:patch Release as a patch semver bump labels Jul 19, 2026

@rickardvh rickardvh left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 15fbaf6: still blocked.

The follow-up resolves several earlier findings in the in-process executor: selector existence is checked before projection, host-supplied discovery/detail commands replace synthesized CLI flags, inventory counts are exact, and custom-kind replacement is limited to the first matching segment.

Remaining blockers:

  1. Selector limits silently alter the request. _projection_selectors() materializes all tokens, then keeps only 32 and truncates each to 256 characters. An unknown 33rd selector can disappear and produce success; an overlong selector is validated under a different name. Limit violations must return an explicit bounded request-validation error rather than silently dropping or mutating input.

  2. Generated Python and TypeScript do not implement the same bounded contract. They still accept unbounded selector counts and lengths, return every requested/unknown selector and suggestion entry, and emit empty discovery/detail commands. Generated Python also still replaces every /selected-output/ segment while the executor and TypeScript replace only the first. The source executor and both generated targets must share one behaviorally identical implementation contract.

  3. The inventory walker remains inventory-sized in memory for wide payloads. _selector_inventory_summary() avoids retaining the final recursive catalog, but constructs complete entries collections for each object/list and pushes all siblings onto pending; TypeScript similarly creates full mapped/reversed arrays. Use iterator-based traversal that retains only traversal depth/state plus the fixed-size sample.

  4. Cross-target conformance is still missing. The added tests exercise the in-process executor, and primitive_conformance.py repeats that same surface. Add one canonical contract case executed through the executor, generated Python runtime, and generated TypeScript runtime, covering limit rejection, atomicity, exact counts, bounded suggestions, host commands, custom kinds, and output-size bounds.

  5. Release classification remains incompatible. Changing the established missing-selector result shape unconditionally is a breaking primitive-contract change. semver:minor is insufficient unless the new behavior is introduced through an opt-in or explicitly versioned contract.

The new head currently has no attached CI/check status, is non-mergeable, and is behind master. Rebase, run the repository-required primitive/public API conformance lanes, and attach exact-head generated-target proof after correcting the remaining contract gaps.

@rickardvh rickardvh added semver:major Release as a major semver bump and removed semver:minor Release as a minor semver bump labels Jul 19, 2026
@rickardvh
rickardvh force-pushed the codex/payload-project-bounded-selector-errors branch from 15fbaf6 to 6342cf3 Compare July 19, 2026 14:17

@rickardvh rickardvh left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 6342cf3: still blocked on two remaining contract issues.

The previous architectural findings are otherwise resolved: selector-request limit violations now fail explicitly, validation precedes projection, the inventory summary retains only a fixed sample, host discovery/detail commands pass through unchanged, generated Python/TypeScript parity coverage exists, CI is green, and semver:major is correct.

Remaining blockers:

  1. The accepted request bounds do not guarantee the upstream 6 KB error-envelope hard cap. The contract accepts up to 32 selectors of 256 characters each. For unknown selectors, the response emits those names in both requested_selectors and unknown_selectors, plus suggestion keys and inventory metadata. Maximum accepted input can therefore produce well over 16 KB of selector-name content alone. The new generated-target test asserts only < 12000 bytes with short selector/sample strings, so it does not prove the actual selected-output error hard cap from AW #1899. Add a cumulative selector-request byte/character budget chosen to guarantee the envelope cap, and also bound host command/sample path contributions or enforce a final construction-time envelope budget. Prove worst-case emitted UTF-8 JSON remains below 6 KB through the executor and both generated targets.

  2. Python and TypeScript define selector length differently for Unicode. Python uses len(token) (Unicode code points), while TypeScript uses token.length (UTF-16 code units). A selector containing 129 astral characters is accepted by Python as length 129 but rejected by TypeScript as length 258. The parity fixture covers only ASCII. Define the contract in one cross-language unit—preferably UTF-8 bytes or Unicode code points—and implement and test it identically in both targets.

After these two points are corrected, the generator-side contract appears ready for the dependent AW regeneration and host-level budget proof.

@rickardvh rickardvh left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 86442cb: the previous two blockers are resolved, but one cross-target determinism issue remains.

What is now correct:

  • selector limits are defined in UTF-8 bytes in the interpreter and generated TypeScript runtime;
  • cumulative selector-name input is capped at 512 bytes;
  • the final validation envelope is measured as compact UTF-8 JSON and fitted below 6 KB;
  • source command, detail commands, selector samples, and suggestions have explicit budgets;
  • generated Python/TypeScript parity tests cover the 6 KB cap, cumulative overflow, and astral Unicode boundaries;
  • exact-head CI and the semver-label check pass, the branch is mergeable, and semver:major remains correct.

Remaining blocker:

The bounded selector sample is traversal-order dependent, so Python and TypeScript can still emit different validation packets for the same JSON payload.

record_sample() checks the cumulative sample-byte budget before adding a candidate. Once the current traversal has filled that budget, a later lexicographically smaller path is rejected rather than replacing a larger retained path. Python traverses mappings in insertion order, while JavaScript object enumeration moves integer-index keys into numeric order.

A concrete shape is an object whose root keys were inserted as "4", "3", "2", "1", "0", with each value containing one nested selector path close to the 96-byte per-path limit. Python visits branches 4→0; TypeScript visits 0→4. With the 384-byte sample budget, each runtime retains nested paths from different branches. selector_inventory.sample therefore differs, and suggestions can differ with it.

This conflicts with the deterministic bounded-sample contract and with the purpose of the generated-target parity fixture.

Required direction: make sample membership independent of traversal order while retaining O(sample-limit) memory. For example, skip individually overlong paths, maintain only the lexicographically smallest sample_limit candidates during traversal, then apply the cumulative byte budget to that sorted bounded candidate set after traversal. Add one executor/generated-Python/generated-TypeScript fixture using integer-like object keys and near-budget nested paths.

No other blockers found on this head.

@rickardvh
rickardvh marked this pull request as ready for review July 19, 2026 14:51

@rickardvh rickardvh left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recheck of d1c7ceb: still blocked on one cross-target determinism issue.

The integer-like key traversal case is fixed, exact-head CI and semver checks pass, the PR is mergeable, and it is now ready for review.

Remaining blocker:

The selector sample still uses language-native string ordering. Python calls sample_candidates.sort(), which orders by Unicode code point. TypeScript calls sampleCandidates.sort(), whose default ordering compares UTF-16 code units. Those differ outside the BMP, so the same JSON payload can still yield different bounded samples—and therefore different suggestions and validation packets—between the interpreter/generated Python runtime and generated TypeScript runtime.

A concrete boundary case is U+E000 versus U+10000: Python orders U+E000 first, while JavaScript orders U+10000 first because its leading surrogate is below U+E000. With eight lower ASCII candidates plus these two paths at the sample cutoff, the retained eighth path diverges.

Define and implement one explicit shared comparator, preferably lexicographic UTF-8 byte ordering to match the rest of this contract, and add that exact non-BMP boundary case to the interpreter/Python/TypeScript conformance fixture. Once that passes, the generator-side contract is ready for the dependent AW regeneration and host-level closure proof.

@rickardvh
rickardvh merged commit ef0512c into master Jul 19, 2026
2 checks passed
@rickardvh
rickardvh deleted the codex/payload-project-bounded-selector-errors branch July 19, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver:major Release as a major semver bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant