Skip to content

XL-2: sequence-operand refs reach resolve - #12156

Closed
briansrls wants to merge 5 commits into
mainfrom
session/jolly-dove-24
Closed

briansrls wants to merge 5 commits into
mainfrom
session/jolly-dove-24

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session jolly-dove-24.
Pushing to session/jolly-dove-24 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

Brian Searls and others added 5 commits September 23, 2026 13:27
…equence operand is never narrowed

body_lower_read_operator_expression reads dag_grammar_binary_expr_expr's tower off the
parse shape and folds every operand left-associatively, with the prefix ! and - lowered
as a unary Transform; an unreadable operand or tail refuses at its position. The
first-match infix search is deleted, and body_lower_operand_ref_optional no longer
returns pair.left for a sequence with a non-empty right. Witness:
v2.test.claim.namespace_xl0.sequence_operand_resolve_refusal (3 candidates red on
main, green here; declared-right and left-only controls green on both).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts:
#	src/v2/compiler/body_lowering_fold.dag
…, not listed beside them (review 70545)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…c specimen does not reproduce on the normalize route

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…-frame grammar build pushed body_lowering_normalize_add over the new-witness eval-step budget

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@briansrls
briansrls marked this pull request as ready for review September 23, 2026 16:13
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-23T16:19:51.287407Z babf511 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: babf511d6d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1296 to +1301
match body_lower_operator_token_optional(node: pair.left, binary: false) {
Present { value: operator } =>
match body_lower_operator_operand(node: pair.right) {
Rejected { diagnostics: r } => OperatorExpressionRefused { diagnostics: r }
Accepted { value: operand, diagnostics: _ } =>
OperatorExpressionRead { lowered: lower_unary_prefix(operator: operator, operand: operand) }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Canonicalize prefix operators by arity

When a parsed -x or !x reaches this new arm, lower_unary_prefix creates a two-child Transform containing the raw token, but the resolver's token-only dag_surface_operator_canonicalization_member maps dag_token_minus to binary subtraction and has no mapping for dag_token_bang. Consequently -x resolves as subtraction with one operand, while !x remains a surface-token/callable shape; the translation path then rejects these because target_project_transform_primitive_apply_in_arrow_scope requires three children. Prefix lowering needs to select canonical_operation_op_negate/canonical_operation_op_not (or otherwise preserve unary arity) rather than reusing binary token canonicalization.

Useful? React with 👍 / 👎.

@gunbai-bot

gunbai-bot Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Closing as a duplicate. The head of this branch is babf511, the exact head that was squash-merged as #12145 (ada85b9). It adds no new commits, and the diff against main is only main having moved on since. Review 70593's approval matches the approval on #12145, and the one remaining gap (body_lower_operand_ref_sequence_optional answering Absent) is already being handled in a separate lane.

— sent from quiet-seal-543

@gunbai-bot gunbai-bot Bot closed this Sep 23, 2026
@gunbai-bot
gunbai-bot Bot deleted the session/jolly-dove-24 branch September 23, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant