Skip to content

Fix release builds#2629

Merged
paul-nechifor merged 4 commits into
mainfrom
sam/fix-release-build
Jun 26, 2026
Merged

Fix release builds#2629
paul-nechifor merged 4 commits into
mainfrom
sam/fix-release-build

Conversation

@Dreamsorcerer

Copy link
Copy Markdown
Collaborator
  • Remove macos-13, now unsupported.
  • Use glibc 2.31+ in build, needed by open3d.
  • Replace annotation-protocol, it declares support for Python <= 3.13.0, making it useless.

Someone more familiar with the reason for using annotation-protocol should probably review and simplify these changes later, I doubt we need this much complexity for the one check (maybe a pydantic check would be enough?).

@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the release build pipeline by removing the deprecated macos-13 runner, upgrading the Linux wheel images to manylinux_2_34 on both x86_64 and aarch64 to satisfy open3d's glibc 2.31+ requirement, and replacing the annotation-protocol dependency (which capped Python support at ≤ 3.13.0) with a hand-rolled annotation-compliance checker backed by typing_extensions/stdlib typing.

  • Workflow: macos-13 removed; CIBW_MANYLINUX_X86_64_IMAGE and CIBW_MANYLINUX_AARCH64_IMAGE both set to manylinux_2_34.
  • dimos/spec/utils.py: annotation-protocol replaced with _annotation_compatible + _signatures_compatible helpers; imports get_protocol_members/is_protocol from typing_extensions on Python < 3.13 and from typing on 3.13+.
  • pyproject.toml: annotation-protocol dep removed; typing_extensions marker widened to python_version < '3.13'; version floor remains >=4.0 even though the new imports require >=4.7.0.

Confidence Score: 4/5

The workflow and dependency removals are correct; the replacement annotation-compliance code is well-structured and tested, but the typing_extensions version floor in pyproject.toml is lower than what the new imports actually require.

The new annotation checker imports get_protocol_members and is_protocol from typing_extensions, both of which were added in 4.7.0, but the declared dependency floor is >=4.0. On Python 3.10 in a clean environment where pip resolves the oldest satisfying version, this will produce an ImportError at runtime. The change otherwise looks solid — manylinux images are correct for both arches, the KeyError guard on defaulted spec parameters is in place, and the new helper logic is covered by targeted tests.

pyproject.toml — the typing_extensions version specifier needs attention to match what the new imports actually require.

Important Files Changed

Filename Overview
.github/workflows/release.yml Removes the end-of-life macos-13 runner; sets CIBW_MANYLINUX_X86_64_IMAGE and CIBW_MANYLINUX_AARCH64_IMAGE to manylinux_2_34, which provides glibc 2.31+ required by open3d and is the correct official PyPA image for both architectures.
dimos/spec/utils.py Replaces annotation-protocol dependency with a custom implementation using typing_extensions (< 3.13) or stdlib typing (>= 3.13); introduces _annotation_compatible and _signatures_compatible helpers with correct handling of optional/extra parameters; treats None as a wildcard spec return annotation.
dimos/spec/test_utils.py Adds test cases covering defaulted spec parameters missing from the impl and extra optional/required parameters in the impl; good coverage of the newly introduced _signatures_compatible logic.
pyproject.toml Removes annotation-protocol dependency; widens typing_extensions marker to python_version < '3.13', but the version floor remains >=4.0 while the new imports (get_protocol_members, is_protocol) require typing_extensions >=4.7.0.
docs/development/releasing.md Minor clarifying note added to step 3, explaining CI must complete on main before a branch push succeeds.
uv.lock Lock file regenerated to reflect removal of annotation-protocol and the updated typing_extensions conditional; exclude-newer timestamp also refreshed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["spec_annotation_compliance(obj, proto)"] --> B{"is_spec(proto)?"}
    B -->|No| C["raise TypeError"]
    B -->|Yes| D{"isinstance(obj, runtime_checkable(proto))?"}
    D -->|No| E["return False"]
    D -->|Yes| F["For each name in get_protocol_members(proto)"]
    F --> G{"spec_sig obtainable?"}
    G -->|No| H["skip - data attribute"]
    H --> F
    G -->|Yes| I{"impl_sig obtainable?"}
    I -->|No| E
    I -->|Yes| J["_signatures_compatible(spec_sig, impl_sig)"]
    J --> K{"return annotations compatible?"}
    K -->|No| E
    K -->|Yes| L["Build args/kwargs from impl params\nskip extra optional params not in spec"]
    L --> M{"spec_sig.bind succeeds?"}
    M -->|No| E
    M -->|Yes| N["For each spec_param"]
    N --> O{"VAR_POSITIONAL or VAR_KEYWORD?"}
    O -->|Yes| P["skip"]
    P --> T
    O -->|No| Q{"param name in bound.arguments?"}
    Q -->|No| E
    Q -->|Yes| R{"name and kind compatible?"}
    R -->|No| E
    R -->|Yes| S{"_annotation_compatible?"}
    S -->|No| E
    S -->|Yes| T{"more params?"}
    T -->|Yes| N
    T -->|No| U["return True"]
    U --> F
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["spec_annotation_compliance(obj, proto)"] --> B{"is_spec(proto)?"}
    B -->|No| C["raise TypeError"]
    B -->|Yes| D{"isinstance(obj, runtime_checkable(proto))?"}
    D -->|No| E["return False"]
    D -->|Yes| F["For each name in get_protocol_members(proto)"]
    F --> G{"spec_sig obtainable?"}
    G -->|No| H["skip - data attribute"]
    H --> F
    G -->|Yes| I{"impl_sig obtainable?"}
    I -->|No| E
    I -->|Yes| J["_signatures_compatible(spec_sig, impl_sig)"]
    J --> K{"return annotations compatible?"}
    K -->|No| E
    K -->|Yes| L["Build args/kwargs from impl params\nskip extra optional params not in spec"]
    L --> M{"spec_sig.bind succeeds?"}
    M -->|No| E
    M -->|Yes| N["For each spec_param"]
    N --> O{"VAR_POSITIONAL or VAR_KEYWORD?"}
    O -->|Yes| P["skip"]
    P --> T
    O -->|No| Q{"param name in bound.arguments?"}
    Q -->|No| E
    Q -->|Yes| R{"name and kind compatible?"}
    R -->|No| E
    R -->|Yes| S{"_annotation_compatible?"}
    S -->|No| E
    S -->|Yes| T{"more params?"}
    T -->|Yes| N
    T -->|No| U["return True"]
    U --> F
Loading

Reviews (3): Last reviewed commit: "Fix" | Re-trigger Greptile

Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml Outdated
Comment thread pyproject.toml
Comment thread dimos/spec/utils.py
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.16854% with 31 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/spec/utils.py 60.86% 15 Missing and 12 partials ⚠️
dimos/spec/test_utils.py 80.00% 4 Missing ⚠️
@@            Coverage Diff             @@
##             main    #2629      +/-   ##
==========================================
- Coverage   71.04%   71.01%   -0.04%     
==========================================
  Files         892      892              
  Lines       79287    79371      +84     
  Branches     7079     7100      +21     
==========================================
+ Hits        56330    56362      +32     
- Misses      21114    21156      +42     
- Partials     1843     1853      +10     
Flag Coverage Δ
OS-ubuntu-24.04-arm 63.33% <65.16%> (-0.01%) ⬇️
OS-ubuntu-latest 66.07% <59.55%> (-0.02%) ⬇️
Py-3.10 66.07% <59.55%> (-0.02%) ⬇️
Py-3.11 66.07% <59.55%> (-0.02%) ⬇️
Py-3.12 66.07% <59.55%> (-0.01%) ⬇️
Py-3.13 66.07% <59.55%> (-0.01%) ⬇️
Py-3.14 66.08% <65.16%> (-0.01%) ⬇️
Py-3.14t 66.07% <59.55%> (-0.02%) ⬇️
SelfHosted-Large 30.06% <49.43%> (+0.01%) ⬆️
SelfHosted-Linux 37.56% <20.22%> (-0.03%) ⬇️
SelfHosted-macOS 36.38% <20.22%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/spec/test_utils.py 87.50% <80.00%> (-4.17%) ⬇️
dimos/spec/utils.py 56.12% <60.86%> (+3.18%) ⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 26, 2026
@paul-nechifor paul-nechifor merged commit f6ac23d into main Jun 26, 2026
26 of 28 checks passed
@paul-nechifor paul-nechifor deleted the sam/fix-release-build branch June 26, 2026 23:29
@github-actions

Copy link
Copy Markdown

Backport branch created but failed to create PR.
Request to create PR rejected with status 403.

(see action log for full response)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport release/0.0.13 ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants