Skip to content

feat(integrations): AppLookupService aggregator — Phase 3 complete#53

Merged
strausmann merged 3 commits into
mainfrom
feat/app-lookup-aggregator
May 11, 2026
Merged

feat(integrations): AppLookupService aggregator — Phase 3 complete#53
strausmann merged 3 commits into
mainfrom
feat/app-lookup-aggregator

Conversation

@strausmann
Copy link
Copy Markdown
Owner

Summary

Phase 3 final PR. Adds the AppLookupService aggregator that routes lookup(source_app, id) calls to the right per-app client (Snipe-IT, Grocy, Spoolman). Phase 3 (App-Lookup-Service) closes here.

The aggregator is dependency-free of its clients via a _LookupClient Protocol — no concrete-class imports, no circular dependencies. New apps (e.g. OpenFoodFacts) plug in by extending _AppName and the constructor.

What's in this PR

backend/app/services/lookup_service.py

  • _LookupClient(Protocol) — minimal async lookup(identifier) -> LabelData contract. All three concrete clients satisfy it structurally.
  • _AppName = Literal["snipeit", "grocy", "spoolman"] — static-analysis aid.
  • AVAILABLE_APPS = get_args(_AppName) — runtime constant derived from the Literal, so adding a new app touches one place.
  • UnknownAppError(Exception) — explicitly does NOT inherit from AppLookupNotFoundError. The two errors are semantically distinct: "misconfigured request" vs "entity doesn't exist".
  • AppLookupService(*, snipeit, grocy, spoolman) — keyword-only init, dict-based dispatch, available_apps precomputed in __init__.

backend/tests/unit/services/test_lookup_service.py — 8 tests

  • Dispatch paths for each of the three clients (verified via assert_awaited_once_with).
  • UnknownAppError with bad app name in message.
  • UnknownAppError lists all available apps.
  • AppLookupNotFoundError from underlying client propagates unchanged.
  • AVAILABLE_APPS and service.available_apps agree (cross-check).
  • Non-inheritance invariant: UnknownAppError is NOT a subclass of AppLookupNotFoundError.

What's NOT in this PR

  • FastAPI endpoint wiring — Phase 6.
  • LabelRenderer, Templates, persistence — Phases 4, 5.

Test plan

  • pytest -q → 111/111 (103 prior + 8 new).
  • ruff format --check . clean.
  • ruff check . clean.
  • mypy app/ (strict) clean — 22 source files.

Review history (subagent-driven)

  1. Implementer 3feae15 — initial commit with Protocol-typed dispatch + Literal + AVAILABLE_APPS + UnknownAppError + 8 tests.
  2. Spec compliance: ✅ — every spec point + intentional improvement (kw-only init, Protocol, Literal, AVAILABLE_APPS, non-inheritance invariant) verified line-by-line.
  3. Code quality: APPROVED_WITH_NITS — flagged duplicated string literals between _AppName and AVAILABLE_APPS, available_apps re-sorting on every call, and absence of docstring note about Literal being doc-only.
  4. Fix commit 29ba1adAVAILABLE_APPS = get_args(_AppName) eliminates duplication, available_apps precomputed as plain attribute, lookup docstring extended.

Phase 3 status

With this merge, the entire Phase 3 App-Lookup-Service layer is in place:

LabelData (schemas)
   ↑
   │ produced by
   │
SnipeITClient → 8 tests   ╮
GrocyClient  → 9 tests    │← all inherit AppLookupNotFoundError
SpoolmanClient → 9 tests  ╯
   ↑
   │ dispatched by
   │
AppLookupService → 8 tests  (Protocol-typed, route by source_app)

Phase 4 (LabelRenderer + TemplateService) builds on LabelData.

Linked plan

docs/superpowers/plans/2026-05-11-label-printer-hub.md Task 3.5 closes Phase 3.

strausmann and others added 2 commits May 11, 2026 10:33
…ispatch

Routes lookup(source_app, identifier) calls to the registered per-app client
(Snipe-IT, Grocy, Spoolman) using a Protocol-typed _LookupClient contract.
UnknownAppError is kept intentionally distinct from AppLookupNotFoundError —
one is a caller configuration mistake, the other is a missing entity.
Keyword-only __init__, AVAILABLE_APPS constant, and available_apps property
support future clients and API-layer discovery without signature changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ute available_apps, document Literal limitation

- Fix A: AVAILABLE_APPS now derived from get_args(_AppName) — eliminates
  string duplication and drift risk between the Literal and the constant.
- Fix B: available_apps converted from @Property (re-sorts on every call)
  to a plain attribute computed once in __init__; _clients is immutable
  after construction so this is always safe.
- Fix C: lookup() docstring extended to clarify that _AppName is for
  static-analysis tooling only and does not restrict runtime callers;
  UnknownAppError covers the runtime mismatch case.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 11, 2026 10:38
@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request completes Phase 3 of the label printer hub project by introducing the AppLookupService aggregator. This service acts as a central dispatcher, routing lookup calls to the appropriate per-application client (Snipe-IT, Grocy, Spoolman) without direct dependencies on their concrete implementations. The design leverages Python's Protocol for flexible client integration and provides clear error differentiation for misconfigured requests versus missing entities, enhancing the system's robustness and extensibility.

Highlights

  • Protocol-based Client Integration: Introduced _LookupClient(Protocol) to define a minimal contract for per-app clients, ensuring dependency-free integration and avoiding circular dependencies by relying on structural subtyping.
  • Dynamic App Management: Implemented _AppName = Literal[...] and AVAILABLE_APPS using get_args for static analysis and runtime management of available applications, simplifying the addition of new apps to a single point of modification.
  • Distinct Error Handling: Defined UnknownAppError to specifically handle configuration mismatches when an unregistered app is requested, clearly distinguishing it from AppLookupNotFoundError which indicates a missing entity within a known app.
  • Robust AppLookupService: Created AppLookupService with keyword-only initialization for its client dependencies and dict-based dispatch, precomputing available_apps at construction for efficiency and immutability.
  • Comprehensive Unit Tests: Added 8 new unit tests covering client dispatch for Snipe-IT, Grocy, and Spoolman, UnknownAppError scenarios (including message content), AppLookupNotFoundError propagation, and consistency checks for AVAILABLE_APPS.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new backend “App Lookup” aggregation layer that dispatches lookup(source_app, identifier) to the appropriate per-integration client (Snipe-IT, Grocy, Spoolman), providing a single entrypoint for Phase 3 of the label lookup pipeline.

Changes:

  • Introduces AppLookupService with Protocol-typed client dispatch, AVAILABLE_APPS, and an UnknownAppError for unregistered apps.
  • Adds unit tests covering routing, error semantics, and invariants (including non-inheritance from AppLookupNotFoundError).

Reviewed changes

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

File Description
backend/app/services/lookup_service.py New aggregator service that routes lookups to the correct integration client and exposes available-app metadata.
backend/tests/unit/services/test_lookup_service.py New unit tests verifying dispatch behavior and error semantics for the aggregator.

Comment thread backend/app/services/lookup_service.py Outdated
Comment on lines +52 to +58
self._clients: dict[str, _LookupClient] = {
"snipeit": snipeit,
"grocy": grocy,
"spoolman": spoolman,
}
# Computed once at construction — _clients never mutates after __init__.
self.available_apps: tuple[str, ...] = tuple(sorted(self._clients))
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the AppLookupService to route lookup requests to specific application clients (Snipe-IT, Grocy, and Spoolman) using an asynchronous protocol, supported by comprehensive unit tests. Feedback identifies opportunities to improve type safety by using the _AppName literal for internal attributes and to optimize an error message by utilizing a pre-computed list of available apps.

Comment thread backend/app/services/lookup_service.py Outdated
Comment on lines +52 to +58
self._clients: dict[str, _LookupClient] = {
"snipeit": snipeit,
"grocy": grocy,
"spoolman": spoolman,
}
# Computed once at construction — _clients never mutates after __init__.
self.available_apps: tuple[str, ...] = tuple(sorted(self._clients))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The type hints for _clients and available_apps can be made more precise by using the _AppName Literal instead of str. This improves type safety and aligns with the project's emphasis on strict typing and mypy compliance.

Suggested change
self._clients: dict[str, _LookupClient] = {
"snipeit": snipeit,
"grocy": grocy,
"spoolman": spoolman,
}
# Computed once at construction — _clients never mutates after __init__.
self.available_apps: tuple[str, ...] = tuple(sorted(self._clients))
self._clients: dict[_AppName, _LookupClient] = {
"snipeit": snipeit,
"grocy": grocy,
"spoolman": spoolman,
}
# Computed once at construction — _clients never mutates after __init__.
self.available_apps: tuple[_AppName, ...] = tuple(sorted(self._clients))
References
  1. The repository style guide prioritizes type safety and strict mypy compliance (Rule 7). (link)

Comment thread backend/app/services/lookup_service.py Outdated
"""
client = self._clients.get(source_app)
if client is None:
raise UnknownAppError(f"Unknown app {source_app!r}. Available: {sorted(self._clients)}")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The error message re-calculates sorted(self._clients), which is redundant as this value is already pre-computed and stored in self.available_apps during initialization. Using the existing attribute is more efficient and maintains consistency within the class.

Suggested change
raise UnknownAppError(f"Unknown app {source_app!r}. Available: {sorted(self._clients)}")
raise UnknownAppError(f"Unknown app {source_app!r}. Available: {self.available_apps}")

…l + reuse available_apps in error message

- _clients dict and available_apps tuple are now typed dict[_AppName, _LookupClient]
  and tuple[_AppName, ...] respectively; string literals in the dict literal satisfy
  mypy without cast at construction
- cast(_AppName, source_app) at the .get() call site bridges the intentionally
  wider str parameter (runtime validation) to the narrower key type
- UnknownAppError message now uses precomputed self.available_apps instead of
  re-sorting self._clients on every error path

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@strausmann strausmann merged commit 222bef4 into main May 11, 2026
9 checks passed
github-actions Bot pushed a commit that referenced this pull request May 12, 2026
## 0.3.0 (2026-05-12)

* feat(config): pydantic-settings module with env-driven runtime configuration (#45) ([878e9e0](878e9e0)), closes [#45](#45)
* feat(integrations): AppLookupService aggregator — Phase 3 complete (#53) ([222bef4](222bef4)), closes [#53](#53)
* feat(integrations): Grocy + Spoolman lookup clients with shared NotFoundError base (#52) ([b1c9c3c](b1c9c3c)), closes [#52](#52)
* feat(integrations): LabelData schema + Snipe-IT lookup client (#51) ([3bc180f](3bc180f)), closes [#51](#51)
* feat(label-renderer): Template schema + Pillow/qrcode renderer for 1-bit label bitmaps (#54) ([fb77028](fb77028)), closes [#54](#54)
* feat(printer-models): Brother PT-Series TapeRegistry with TZe and heat-shrink specs (#47) ([7526019](7526019)), closes [#47](#47)
* feat(printer-models): Job lifecycle FSM with explicit state machine (#49) ([1a8c40e](1a8c40e)), closes [#49](#49)
* feat(printer-models): PrinterModel Protocol + ModelRegistry for plugin discovery (#48) ([2ae0e09](2ae0e09)), closes [#48](#48)
* feat(printer-models): PrintQueue worker with pause/resume/cancel/retry (#50) ([dfdf6fe](dfdf6fe)), closes [#50](#50)

[skip ci]
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.

2 participants