chore: pin the optionality-parity landing - #98
Open
dlipicar wants to merge 1 commit into
Open
Conversation
logos-qt-sdk af767b7 → 3cd5297 (#29 optional params on every Qt surface, #30 goldens are valid C++, #31 jsonReturn shapes are already Qt types) logos-rust-sdk be0deda → 0b4b8ed (#35 Option<T> in the Rust-first frontend, #36 admit ?any + scope the optional-return refusal, #37 refresh roundtrip goldens, #38 typed event emitters speak records) logos-module-builder 5a400e3 → ddddd8c (#187/#188 re-pin the two SDKs) logos-test-modules fb773a5 → f8077fa (#43 typed record emitter call site, and the SDK triple now sourced from module-builder) Completes `?T` optionality: it was already implemented in logos-lidl, the C++ generators and the Rust provider/consumer; this lands the last two surfaces — the Qt generators, and `Option<T>` in the Rust-first authoring direction. Optional RETURNS stay refused on the Qt path, where json null is already how a FAILED call is reported (logos_json_convert → invalid QVariant → METHOD_FAILED), so an empty `?T` would be indistinguishable from a failure. They remain supported on the cdylib path, which is what test_fullapi_ext_{rust,cpp} and the logoscore-py client exercise. Four pre-existing bugs fixed along the way: * unchecked QList::at in the Qt provider dispatch — a caller sending too few arguments read past the end; now args.value() plus an explicit arity gate * `int`/`uint` returns cast through `static_cast<int>` — they are int64_t and uint64_t, so every value past 2^31 was silently truncated * jsonReturn shapes double-converted — `any` was forced through `.toList()`, coercing an object or scalar into a list * logos-test-modules sourced its SDK triple from logos-liblogos.inputs while compiling code the BUILDER's generator emitted, so the two pins drifted and the unit tests stopped compiling the moment cpp-sdk#132 taught the generator to emit `#include "logos_async_result.h"` Both flake.lock and the submodule gitlinks are bumped: the doctest job pins each submodule to its gitlink via --release-for, so a lock-only bump would leave the doctest side testing the old code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the workspace’s pinned dependency revisions to land the remaining ?T optionality parity surfaces by bumping the relevant SDK and test-module inputs in the root flake, ensuring the workspace evaluates against the intended upstream commits.
Changes:
- Bump
logos-qt-sdk,logos-rust-sdk,logos-module-builder, andlogos-test-modulesinput URLs inflake.nix. - Refresh
flake.lockto reflect the new locked revisions (and updated transitive locks such aslogos-lidlwhere applicable).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| flake.nix | Updates the flake input URLs to the new upstream commits for qt-sdk, rust-sdk, module-builder, and test-modules. |
| flake.lock | Updates locked revisions/hashes to match the new workspace pins and their transitive dependency graph. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Completes
?Toptionality. It was already implemented in logos-lidl, the C++ generators and the Rust provider/consumer; this lands the last two surfaces — the Qt generators, andOption<T>in the Rust-first authoring direction.logos-qt-sdkaf767b73cd5297logos-rust-sdkbe0deda0b4b8edOption<T>in the Rust-first frontend · #36 admit?any+ scope the optional-return refusal · #37 refresh roundtrip goldens · #38 typed event emitters speak recordslogos-module-builder5a400e3ddddd8clogos-test-modulesfb773a5f8077faOptional returns stay refused on the Qt path only
An empty
?Tis JSON null, and null is already how a failed call is reported there (logos_json_convert→ invalidQVariant→METHOD_FAILED), so "found nothing" would be indistinguishable from "the call failed".They remain supported on the cdylib path — cpp-sdk keeps them eligible by design, and
test_fullapi_ext_{rust,cpp}plus the logoscore-py client exerciseechoOptional(v: ?tstr) -> ?tstras a cross-language conformance case.Four pre-existing bugs fixed along the way
QList::atin the Qt provider dispatch — a caller sending too few arguments read past the end. Nowargs.value()plus an explicit arity gate.int/uintreturns cast throughstatic_cast<int>— they areint64_t/uint64_t, so every value past 2³¹ was silently truncated.jsonReturnshapes double-converted —anywas forced through.toList(), coercing an object or scalar into a list.logos-liblogos.inputswhile compiling code the builder's generator emitted. The two pins drifted independently, and the unit tests stopped compiling the moment cpp-sdk#132 taught the generator to emit#include "logos_async_result.h".Both halves bumped
flake.lockand the submodule gitlinks. The doctest job pins each submodule to its gitlink via--release-for, so a lock-only bump would leave the doctest side testing the old code.Lock revs verified at the root level (
nodes[root].inputs), not by node name — flake.lock keys are deduplicated, so a node calledlogos-qt-sdkis not necessarily the one the root resolves.Verification
Every component PR is merged and green. Raised for the full Tests and Doc-tests run, which is
workflow_dispatch-only and does not trigger on PR open — it needs to be launched against this branch.🤖 Generated with Claude Code