feat(service): allow specifying D-Bus signatures via decorator arguments

[bluetoothbot]

What

Let @dbus_method, @dbus_signal, and @dbus_property take the D-Bus signature directly as a decorator argument, instead of only via parameter/return annotations.

Why

Closes #438. Today the only way to declare a member's signature is through annotations — either bare string literals (val: 's') or typing.Annotated[..., DBusSignature(...)]. The bare-string form makes mypy/pyright flag every annotation, and the Annotated form is verbose. Some callers also just don't want type hints at all. The maintainer flagged this as "a reasonable change" on the issue; this mirrors the original python-dbus-next design (altdesktop/python-dbus-next#111).

@dbus_method(in_signature="su", out_signature="s")
def echo_two(self, one: str, two: int) -> str:
    return f"{one}{two}"

@dbus_signal(signature="as")
def list_signal(self) -> list[str]: ...

@dbus_property(signature="u")
def int_prop(self) -> int: ...

How

• in_signature/out_signature on dbus_method, signature on dbus_signal/dbus_property. When provided, the corresponding annotation is not inspected; when omitted, behaviour is unchanged.
• Method IN-arg names still flow into introspection by zipping the explicit signature's complete types with the function's parameter names.
• _Property needed care: property.setter() recreates the instance through __init__ without the options, so the getter's plain type hint can't be parsed there. The explicit signature is stashed in options and restored in set_options() (which setter() calls), with the getter-annotation parse made non-fatal on that recreation path.
• Fully backward compatible — no .pxd change (the new args live on __init__/decorator defs, not the cdef field layout).

Testing

• New tests/service/test_decorator_signatures.py: unit coverage for method/signal/property signatures + introspection arg names/XML, type validation, and a real session-bus round trip (method call + property Get).
• Full suite green on the pure-Python leg (371 passed) and the REQUIRE_CYTHON=1 compiled leg (service is in TO_CYTHONIZE).

---

Quality Report

Changes: 12 files changed, 328 insertions(+), 269 deletions(-)

Code scan: clean

Tests: failed (FAILED)

Branch hygiene: clean

Generated by Kōan

[codecov]

Codecov Report

❌ Patch coverage is 98.14815% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.31%. Comparing base (e2f7ce9) to head (43ba5f7).

Files with missing lines	Patch %	Lines
src/dbus_fast/service.py	98.14%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #743      +/-   ##
==========================================
+ Coverage   92.19%   92.31%   +0.11%     
==========================================
  Files          29       29              
  Lines        3614     3643      +29     
  Branches      621      635      +14     
==========================================
+ Hits         3332     3363      +31     
+ Misses        165      164       -1     
+ Partials      117      116       -1     

☔ 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.

[codspeed-hq]

Merging this PR will not alter performance

✅ 16 untouched benchmarks

---

_{Comparing bluetoothbot:koan/decorator-signatures (43ba5f7) with main (e2f7ce9)}

[bluetoothbot]

Added test(service) coverage for the three decorator-signature validation error paths the codecov/patch check flagged: a method without in_signature whose parameter lacks an annotation, a property signature that isn't a single complete type, and a property with neither a decorator signature nor a return annotation. Patch coverage gap closed.

[Copilot]

Pull request overview

This PR enhances the dbus_fast.service decorators to allow specifying D-Bus signatures directly in decorator arguments (for methods, signals, and properties), enabling users to keep regular Python type hints (or no type hints) without relying on D-Bus signature annotations.

Changes:

• Add in_signature/out_signature parameters to @dbus_method and signature to @dbus_signal / @dbus_property, bypassing annotation parsing when provided.
• Adjust service member construction to build introspection data from explicit decorator signatures (including IN-arg names for methods).
• Add new unit/integration tests covering decorator-provided signatures, validation, introspection XML output, and a session-bus round trip.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/dbus_fast/service.py	Adds decorator arguments for explicit signatures and updates _Method, _Signal, and _Property construction/validation paths accordingly.
tests/service/test_decorator_signatures.py	Adds tests validating explicit decorator signatures for methods/signals/properties, including introspection XML and an async round-trip call.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bluetoothbot]

PR Review — feat(service): allow specifying D-Bus signatures via decorator arguments

Solid, well-scoped feature that closely mirrors the upstream python-dbus-next design — but a recreation-path edge case breaks its primary use case under PEP 563.

Strengths worth calling out:

• The backward-compat story is genuinely careful: annotation paths are untouched when the new args are omitted, and .pxd is correctly left alone since the new args live only on __init__/decorator defs, not the cdef field layout.
• The _Property setter-recreation handling (stash signature in options, restore in set_options() before building introspection) is the right place to solve a genuinely awkward CPython property.setter behavior.
• Method IN-arg names are preserved into introspection by zipping the explicit signature against parameter names — a nice touch that keeps XML output complete.
• Test coverage spans unit, introspection XML, type validation, and a real session-bus round trip.

Needs attention before merge:

• warning feat: speed up unmarshaller #1: writable -> int getter + from __future__ import annotations crashes at import — "int" matches the D-Bus signature regex and _set_signature raises uncaught in the recreation else branch. The tests don't use future-annotations, so this is masked.
• warning chore: fix docs #2: explicit in_signature arity isn't validated against the parameter count; mismatches surface as a TypeError at dispatch instead of a clear error at decoration.
• suggestion: round-trip test should await wait_for_disconnect() like the rest of the suite.

Note: the quality report flags tests as FAILED while the PR claims green — worth confirming locally whether that's the known flaky test_fd_passing segfault or a real regression.

---

🟡 Important

1. PEP 563 + `-> int` getter crashes the setter recreation path
src/dbus_fast/service.py:400-405

The setter-recreation branch wraps only parse_annotation in try/except ValueError, but _set_signature() is called outside that guard. This breaks the feature's primary use case when the user's module has from __future__ import annotations (PEP 563) active — which is extremely common, including dbus-fast's own modules.

With PEP 563, the getter's return annotation arrives as a string, not a type object. For -> int that string is "int", and parse_annotation matches it against its D-Bus-signature regex ^[ybnqiuxtdsoga()v{}h]+$ — i (int32), n (int16), t (uint64) are all valid type codes — so it returns "int" as a three-type signature instead of raising. _set_signature("int") then raises ValueError("the property signature must be a single complete type"), uncaught, propagating out of .setter at class-definition (import) time.

Why it matters: the whole point of this PR is to let callers write plain Python hints like -> int on a decorator-signature property. A writable int property (the most common numeric property) under PEP 563 — the recommended modern style — crashes on import. The new test suite masks this because test_decorator_signatures.py has no from __future__ import annotations, so the annotation arrives as the int type object (which correctly raises and is caught).

Fix — catch ValueError from _set_signature too, since set_options() restores the real signature on the recreation path anyway:

else:
    try:
        signature = parse_annotation(inspection.return_annotation, module)
        if signature:
            self._set_signature(signature)
    except ValueError:
        pass

Please also add a test module that does from __future__ import annotations with a writable @dbus_property(signature="u") -> int to pin this.

        else:
            try:
                signature = parse_annotation(inspection.return_annotation, module)
            except ValueError:
                signature = ""
            if signature:
                self._set_signature(signature)

2. Explicit `in_signature` arity not checked against parameter count
src/dbus_fast/service.py:83-99

When in_signature is supplied, _Method.__init__ zips the signature's complete types against the function parameters but never checks that the counts agree (the else branch just truncates names via param_names[i] if i < len(param_names) else None).

A mismatch is silent here and only surfaces at dispatch time: _callback_method_handler calls method.fn(interface, *args) (message_bus.py:886) with args derived from the wire body, whose length matches the registered in_signature. If the signature declares fewer/more complete types than the function has parameters, that call raises TypeError far from the decorator, deep inside the message handler.

This aligns with Copilot's inline note. A fail-fast check at decoration time turns a confusing runtime failure into a clear definition-time error:

else:
    param_names = [p.name for i, p in enumerate(inspection.parameters.values()) if i != 0]
    types = get_signature_tree(in_signature).types
    if len(types) != len(param_names):
        raise ValueError(
            f"in_signature has {len(types)} complete types but the method takes "
            f"{len(param_names)} parameters"
        )
    for i, type_ in enumerate(types):
        in_args.append(intr.Arg(type_, intr.ArgDirection.IN, param_names[i]))

Unverified: I did not confirm whether the surrounding handler catches this TypeError and converts it to a D-Bus error reply versus letting it escape — but either way the early check is the better contract.

for i, type_ in enumerate(get_signature_tree(in_signature).types):
    arg_name = param_names[i] if i < len(param_names) else None
    in_args.append(intr.Arg(type_, intr.ArgDirection.IN, arg_name))

🟢 Suggestions

1. Round-trip test doesn't await disconnect
tests/service/test_decorator_signatures.py:176-178

bus1.disconnect() / bus2.disconnect() are issued without awaiting completion. Every other async disconnect in this repo follows await asyncio.wait_for(bus.wait_for_disconnect(), timeout=1) (e.g. tests/test_request_name.py:72-73, tests/test_big_message.py:48-49).

Skipping it can leak the transport/reader tasks past the test, which shows up as cross-test flakiness or failures under strict event-loop cleanup. Matches Copilot's inline note — worth adopting the repo pattern for consistency.

    bus1.disconnect()
    bus2.disconnect()

---

Checklist

• Backward compatibility (omitted args preserve existing behavior)
• Cython .pxd kept in sync with signature changes
• Edge cases covered (PEP 563 future annotations) — warning ＃1
• Input validation at boundaries (signature arity) — warning ＃2
• Test isolation / resource cleanup — suggestion ＃3
• No hardcoded secrets
• Tests verify observable behavior, not source text

---

To rebase specific severity levels, mention me: @bluetoothbot rebase critical (fixes 🔴 only), @bluetoothbot rebase important (fixes 🔴 + 🟡), or just @bluetoothbot rebase for all.

---

Silent Failure Analysis

🟡 **MEDIUM** — silent mismatch tolerated
src/dbus_fast/service.py:75-82

Risk: When the explicit in_signature declares more types than the function has parameters, the extra args are silently given name=None instead of raising, so a signature/parameter mismatch (a real config bug) sails through and only surfaces as a confusing failure at call/dispatch time.

for i, type_ in enumerate(get_signature_tree(in_signature).types):
    arg_name = param_names[i] if i < len(param_names) else None
    in_args.append(intr.Arg(type_, intr.ArgDirection.IN, arg_name))

Fix: Validate that len(get_signature_tree(in_signature).types) == len(param_names) and raise a ValueError on mismatch rather than padding with None.

🟡 **MEDIUM** — swallowed exception
src/dbus_fast/service.py:398-407

Risk: The bare except ValueError swallows every parse failure (not just the expected 'plain hint on the setter-recreation path'), so a genuinely malformed return annotation becomes an empty signature and the property silently ends up with no .signature/.type, deferring the failure to an obscure later AttributeError or wrong introspection.

try:
    signature = parse_annotation(inspection.return_annotation, module)
except ValueError:
    signature = ""
if signature:
    self._set_signature(signature)

Fix: Narrow the swallow to the known recreation case (e.g. only when no annotation is present) or assert that set_options() subsequently restored a signature, raising if neither path produced one.

---

Automated review by Kōan (Claude) HEAD=be07359 5 min 3s

[bluetoothbot]

Rebase with requested adjustments

Branch koan/decorator-signatures was rebased onto main and review feedback was applied.

Changes applied

• Both review items addressed.

• Summary

• src/dbus_fast/service.py — Added up-front arity validation in _Method.__init__: when an explicit in_signature is given, raise ValueError if the number of complete types differs from the non-self parameter count, instead of silently padding extra args with name=None. Per Copilot's inline comment and the silent-failure note, this fails fast at decoration time rather than surfacing later as a dispatch-time TypeError that could escape the message handler.
• tests/service/test_decorator_signatures.py — Awaited disconnect completion in test_decorator_signature_round_trip: added import asyncio and await asyncio.wait_for(bus.wait_for_disconnect(), timeout=1) for both buses after disconnect(), matching the repo's established pattern (tests/test_request_name.py, tests/test_big_message.py) to avoid leaked tasks/transports and event-loop-cleanup flakiness.

Stats

2 files changed, 324 insertions(+), 38 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=No commit on main adds in_signature/out_signature/signature decorator arguments to dbus_method/dbus_)
• Rebased koan/decorator-signatures onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/decorator-signatures to origin
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan