ps-grpc-01-surface: gRPC PlateSolver service surface#84
Conversation
…te swap, and acceptance tests Implements the public gRPC surface for the plate solver: - PlateSolver service with ExtractCentroids, SolveFromCentroids, SolveFromImage, and GetInfo. - Protobuf messages matching cedar-detect Image/ImageCoord shapes and the project Solution contract. - Coordinate swap between wire (x,y) and solver (y,x) at the service boundary. - Shared-memory fast path stub returning INTERNAL to trigger inline fallback. - Detection parameters forwarded to star-detection; solve params forwarded to plate-solver. - Acceptance tests using an in-memory duplex channel client. Fixes during bring-up: - Synthetic test images now use isolated bright pixels on a non-zero background so the detector's strict 1-D peak gate fires. - now reports in degrees on timeout/failure paths (it is already degrees from ) while still converting radians to degrees on the success path. Co-Authored-By: Claude <noreply@anthropic.com>
bryantharpe
added
the
integrity:graded
|
bryantharpe
added
the
hold:human
…mings, model unset attitude Follow-up on the service surface, correcting defects found while verifying the implementation against the spec's acceptance scenarios. Solution FOV was reported in the wrong unit for narrow-field databases. `Solution.fov_used` carries two different units depending on which path produced it: the success path in `refine` copies `PinholeCamera.fov` (radians), while every failure path copies `ctx.fov_initial`, which derives from the database FOV bounds (degrees). The previous conversion guessed the unit by magnitude — "greater than pi must already be degrees". A radian FOV never exceeds pi, so that test only ever misfires on the failure path, where it silently double-converts any database narrower than 3.14 degrees: a 2.5 degree FOV was reported as 143.2. Discriminate on `camera` instead, which is set by exactly the one site that writes radians. Covered by a regression test that fails against the old heuristic with the 143.2 value. `t_extract_ms` and `t_solve_ms` were hardcoded to 0.0 despite the spec requiring the Solution carry them. SolveFromCentroids now reports a measured solve time. SolveFromImage reports its measured total under `t_solve_ms`: the core `solve_from_image` fuses detection and solving into one call, so the two cannot be timed apart without duplicating detection, and ps-grpc-02 owns that RPC's detection path. Reporting the honest total beats inventing a split. The spec calls for unset attitude fields on failure, which a plain proto3 double cannot express — 0.0 is a legitimate ra/dec/roll, so a failed solve was indistinguishable from a real solve on the celestial origin. The attitude and quality fields are now `optional`, matching the core solution's own `Option` modelling. `MATCH_FOUND = 0` meant a default-constructed or absent Solution decoded as a successful match. Added `SOLVE_STATUS_UNSPECIFIED = 0` and renumbered, keeping the five statuses the spec names. Also drops an if/else whose branches both yielded None. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…og data, address review nits
cargo-machete flagged `prost` and `tonic-prost` as unused. Verified empirically
by removing each and rebuilding: both are load-bearing. The code generated by
`tonic-prost-build` into OUT_DIR emits `prost::` paths, `#[prost(..)]`
attributes and `tonic_prost::` codecs, which machete cannot see because it scans
hand-written source only. Kept both and declared them in
[package.metadata.cargo-machete] with the reason.
Matched-star data was wrong in two ways, not just the flagged `mag: 0.0`.
`Solution::matched_catalog_ids` holds database star-table INDICES — verification
feeds them straight to `db.star_vector(StarId(..))` — so emitting them as
`cat_id` leaked an internal index onto the wire as though it were a catalog
identifier. Both magnitude and the real identifier are recoverable from the
database, so `to_proto_solution` now takes it and resolves them. Magnitude comes
from the star table and is optional, since a database need not carry an entry.
BSC and Hipparcos IDs are single numbers; Tycho is a (TYC1, TYC2, TYC3) triple
that cannot be flattened into one integer without inventing an encoding, so a
`cat_id_str` field carries the canonical rendering for every catalog and
`cat_id` is left unset for Tycho.
Remaining review nits:
- The `0.002` passed to the solver is now MATCH_MAX_ERROR, documented as the
spec default and as deliberately not client-facing: it describes how the
database was built, not a per-request knob.
- `(*self.db).clone()` clones the whole pattern database per request. Left in
place with a comment: the solver takes the database by value, so removing the
clone means changing `plate_solver::solve_*` to accept a reference, which is
ps-plate-04's API rather than this boundary's.
- Removed crates/grpc-service/proto/cedar_detect.proto. It was never compiled by
build.rs and never imported, and was byte-identical to proto/cedar_detect.proto
which ps-plate-04 already put at the repo root. A third uncompiled copy inside
the crate's proto directory invites edits that silently do not affect the wire
format.
- The redundant `if return_binned { None } else { None }` was already collapsed
in 51bccf7.
Tests: matched-star resolution (magnitude, real Hipparcos number rather than the
index, and the coordinate swap on matched centroids), Tycho string-only form,
and suppression when return_matches is false.
Verified by hand: cargo fmt --all --check, cargo clippy --locked --all-targets
-- -D warnings, cargo test --locked, cargo machete all clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bryantharpe
added
integrity:cleared
|
1 participant
Implements the public gRPC surface for the plate solver with four RPCs, coordinate swap, shared-memory stub, and acceptance tests.\n\n🤖 Generated with Claude Code