Skip to content

fix(data): bind promoted constructor parameters - #92

Merged
Roger-luo merged 1 commit into
Roger-luo:mainfrom
ChrisRackauckas-Claude:fix/bind-promoting-constructor-parameters
Sep 21, 2026
Merged

Roger-luo merged 1 commit into
Roger-luo:mainfrom
ChrisRackauckas-Claude:fix/bind-promoting-constructor-parameters

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Ignore this draft until reviewed by @ChrisRackauckas.

What changed and why

Promoting constructors added by #76 use a union between the resolved self type and the singleton-bottom type. When no ordinary field binds every ADT parameter, the all-bottom dispatch path leaves one or more method type parameters unbound. This is observable both through Test.detect_unbound_args and, for partially inferred multi-parameter ADTs, as an UndefVarError during construction.

This change emits the broad union constructor only when ordinary fields bind every parameter. Otherwise a single self-reference keeps its exact constructor, while multiple self-references use disjoint anchored promotion methods: earlier arguments must be singleton-bottom, the anchor binds the ADT parameters, and later arguments accept either the resolved or singleton-bottom type.

Failing before

On unmodified main at dff7f45:

TMPDIR="$PWD/.tmp" julia +1.10 --project=. -e 'include("test/data/emit/singleton_promote.jl")'

generated constructors bind type parameters: Test Failed
Evaluated: isempty(Method[... Lst.Cons(...) where T ...])
Test Summary:                               | Fail  Total
generated constructors bind type parameters |    1      1

A standalone two-parameter Moshi reproducer on the same base produced:

unbound_count=1
construction_error=UndefVarError: UndefVarError(:S)

Passing after

TMPDIR="$PWD/.tmp" julia +1.10 --project=. -e 'using Pkg; Pkg.test()'
data            | 324 passed
match           | 140 passed
derive          |  30 passed
perf regression |   5 passed
Testing Moshi tests passed

TMPDIR="$PWD/.tmp" julia +1.12 --project=. -e 'include("test/data/emit/singleton_promote.jl")'
focused generated-constructor tests | 17 passed

TMPDIR="$PWD/.tmp" julia +1.13 --project=. -e 'include("test/data/emit/singleton_promote.jl")'
focused generated-constructor tests | 17 passed

The focused tests include zero detected unbound parameters and zero detected ambiguities.

Downstream verification

SymbolicUtils was tested with its Moshi cap removed and this branch developed locally:

TMPDIR="$PWD/.tmp" GROUP=QA julia +1.10 --project=. -e 'using Pkg; Pkg.test()'
SciMLTesting QA | 18 passed, 18 total
AdjView JET     | 37 passed, 37 total
Testing SymbolicUtils tests passed

Formatting was applied with JuliaFormatter 2.13.0 to both changed files. git diff --check upstream/main passed. git diff upstream/main | typos --config .tmp/typos.toml - passed with a temporary exact-identifier allowance for the pre-existing internal name is_inferrable.

Review considerations

The anchored signatures preserve promotion when a concrete self-reference exists without leaving an all-bottom union branch unbound. A partially inferred multi-parameter all-bottom call now fails dispatch with MethodError instead of entering an invalid method and raising UndefVarError; choosing values for absent type parameters is intentionally left outside this focused fix.

Documentation was not built because this changes no public API or documentation. Nightly Julia and non-Linux platforms were not run locally.

Links

🤖 Generated with Codex (version unknown; model: unknown; session: local session ID 01a04608-d980-7e61-88d9-f6486b0c1916)

@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

@ChrisRackauckas is attempting to deploy a commit to the roger-luo's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Benchmark Results (Julia v1)

Time benchmarks
main b6d7f41... main / b6d7f41...
adt_transform/transform n=100 6.77 ± 0.17 μs 7.09 ± 0.15 μs 0.955 ± 0.031
adt_transform/transform n=1000 0.0668 ± 0.0011 ms 0.0689 ± 0.00083 ms 0.968 ± 0.02
linked_list/sum n=100 0.471 ± 0.01 μs 0.471 ± 0.001 μs 1 ± 0.021
linked_list/sum n=1000 5.12 ± 0.031 μs 5.18 ± 0.061 μs 0.988 ± 0.013
time_to_load 0.0774 ± 0.0015 s 0.079 ± 0.0017 s 0.979 ± 0.028
Memory benchmarks
main b6d7f41... main / b6d7f41...
adt_transform/transform n=100 0.204 k allocs: 6.34 kB 0.204 k allocs: 6.34 kB 1
adt_transform/transform n=1000 2.02 k allocs: 0.0619 MB 2.02 k allocs: 0.0619 MB 1
linked_list/sum n=100 0 allocs: 0 B 0 allocs: 0 B
linked_list/sum n=1000 0 allocs: 0 B 0 allocs: 0 B
time_to_load 0.149 k allocs: 11.1 kB 0.149 k allocs: 11.1 kB 1

@ChrisRackauckas-Claude
ChrisRackauckas-Claude force-pushed the fix/bind-promoting-constructor-parameters branch from e49c732 to 8302f97 Compare August 28, 2026 10:30
Generate anchored promotion methods when self-references are the only source of ADT parameters. Preserve all-bottom and partially inferred construction without leaving method type variables unbound.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: OpenAI Codex <noreply@openai.com>
Agent-Harness: Codex (version unknown)
Agent-Model: unknown
Agent-Session: local session ID 01a04608-d980-7e61-88d9-f6486b0c1916
@ChrisRackauckas-Claude
ChrisRackauckas-Claude force-pushed the fix/bind-promoting-constructor-parameters branch from 8302f97 to b6d7f41 Compare August 28, 2026 10:40
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor Author

Benchmark red-team result: the first 117-line implementation showed a repeatable ~7% package load-time regression, so I narrowed the generator change to 48 added lines and removed the extra partial-inference machinery. The final benchmark at #92 (comment) reports time_to_load 77.4 ± 1.5 ms on main versus 79.0 ± 1.7 ms on this branch (main / branch = 0.979 ± 0.028) with identical allocations. The benchmark workflow passed.

The normal CI workflow is awaiting maintainer approval for this forked PR at https://github.com/Roger-luo/Moshi.jl/actions/runs/33164273641; it has not executed or failed.

@ChrisRackauckas

Copy link
Copy Markdown
Contributor

@Roger-luo what do you think?

@vercel

vercel Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
moshi-jl Ignored Ignored Preview Sep 21, 2026 4:39pm UTC

@Roger-luo
Roger-luo marked this pull request as ready for review September 21, 2026 16:42
@Roger-luo
Roger-luo merged commit 8fe64d3 into Roger-luo:main Sep 21, 2026
6 checks passed
@Roger-luo

Copy link
Copy Markdown
Owner

Thanks. Sorry, I was on vacation and was not checking GitHub.

@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.99%. Comparing base (dff7f45) to head (b6d7f41).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/data/emit/cons.jl 96.15% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #92      +/-   ##
==========================================
+ Coverage   91.91%   91.99%   +0.07%     
==========================================
  Files          43       43              
  Lines        1732     1748      +16     
==========================================
+ Hits         1592     1608      +16     
  Misses        140      140              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions Bot referenced this pull request Sep 27, 2026
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
ChrisRackauckas added a commit to JuliaSymbolics/SymbolicUtils.jl that referenced this pull request Oct 1, 2026
Moshi 0.3.10–0.3.12 emit an unbound promoting Div constructor; v0.3.13
(Roger-luo/Moshi.jl#92) binds those parameters. Keep excluding the broken
range while admitting the fixed release.



Agent-Harness: Cursor Agent CLI 2026.09.28-64d2043
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/agent-jobs/ib-su/SymbolicUtils.jl/jobs/1037-cursor/log.txt on amdci2.julia.csail.mit.edu

Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
Co-authored-by: Cursor Agent <noreply@cursor.com>
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.

3 participants