feat: add Roboflow Inference validator (#13) - #42
Merged
Conversation
PROJECT.md gains a "Post-v1.1 Scope — v1.2" section capturing the brainstormed v1.2 decisions: three sequential PRs covering #13 (Roboflow Inference, self- hosted only), #14 (DOODS2 with operator-selectable HTTP/gRPC transports), and #23 (per-action `ParallelValidators: true` with strict-AND aggregation, default-false for backward compat). ROADMAP.md gains a Phase 14 entry mirroring Phase 13's structure (Goal / Dependencies / Risk per PR / Deliverables / Verification / Success criteria), plus four open questions surfaced for the planner: DOODS2 .proto sourcing, Roboflow Testcontainers feasibility, ParallelValidators flag-location precedence, and aggregate-counter logging in parallel mode. Phase Count footer rolled 13 → 14. STATE.json + HISTORY.md updated. Next: /shipyard:plan 14. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8 plans across 3 waves covering issues #13/#14/#23: - Wave 1 (PR #13 Roboflow): PLAN-1.1 scaffold + DI; PLAN-1.2 8 WireMock tests + CHANGELOG - Wave 2 (PR #14 DOODS2): PLAN-2.1 dual-transport scaffold + vendored .proto + DI; PLAN-2.2 7 HTTP tests; PLAN-2.3 5 gRPC tests + dep-containment verification (gRPC stays plugin-local) - Wave 3 (PR #23 ParallelValidators): PLAN-3.1 ActionEntry/DispatchItem field + converters; PLAN-3.2 dispatcher branch + 6 unit tests; PLAN-3.3 end-to-end integration test + CHANGELOG 21 tasks total, 1570 plan-lines. Test target: 242 baseline → 269 final. VERIFICATION = PASS (all deliverables covered, no plan exceeds 3 tasks, no intra-wave file conflicts). CRITIQUE = READY (all referenced files/line ranges exist; verify commands syntactically valid; gRPC test harness flagged with documented Kestrel fallback for builder). CONTEXT-14 captures user decisions D1-D6 (PR sequencing, Roboflow self-hosted-only, DOODS2 dual-transport, per-action ParallelValidators, strict-AND no-short-circuit) and OQ-1/3/4 resolutions. OQ-2 (Roboflow Testcontainers) resolved by researcher: NOT VIABLE (16.7GB image) → WireMock-only with manual-smoke recipe in PR-1 CHANGELOG. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…se DTOs, Validator)
… keyed-singleton ritual
… tests, 0 warnings)
…ventId assertions)
- Add `global using FluentAssertions;` to Usings.cs; remove redundant file-level `using FluentAssertions;` from RoboflowValidatorTests.cs. Closes the SUMMARY-1.2 / Usings.cs drift the reviewer flagged. - Test 8 (cancellation) now asserts WireMock.LogEntries empty so a future regression that moves the cancellation check after the HTTP call would fail the test. - Add Test 9: ValidateAsync_HttpServerError_FailOpen_ReturnsAllow, closing the HttpRequestException -> FailOpen coverage gap. - Test 1 now asserts Verdict.Score = 0.92 (pins score-forwarding). 9/9 Roboflow tests, 251/251 total, 0 warnings.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughPhase 14 scope is formalized through comprehensive planning documentation (PROJECT.md, ROADMAP.md, context, critique, research, and verification files), with eight sequential plans across three waves. Concurrently, Wave 1 Plan 1.1 is fully implemented: a new Roboflow Inference validator plugin (HTTP-based, self-hosted) with per-instance configuration, DI wiring, and comprehensive unit tests, integrated into the solution and host bootstrap. ChangesPhase 14 Planning & Governance
Wave 1 Plans & Results
Roboflow Plugin Implementation (Wave 1)
Sequence Diagram(s)sequenceDiagram
participant Client as ChannelActionDispatcher
participant EventPump
participant RobofReg as RoboflowPluginRegistrar
participant HttpClient
participant RoboflowInference as Roboflow Server
participant Validator as RoboflowValidator
Client->>EventPump: dispatch action (ActionEntry.ParallelValidators=false)
EventPump->>EventPump: create DispatchItem (ParallelValidators propagated)
EventPump->>Validator: RunValidatorsSequentiallyAsync()
Validator->>Validator: pre-resolve EventContext, SnapshotContext
Validator->>Validator: iterate each IValidationPlugin
Validator->>Validator: invoke ValidateAsync(ctx, snapshot, ct)
Validator->>Validator: build base64 image payload
Validator->>HttpClient: POST /infer/object_detection
HttpClient->>RoboflowInference: JSON request (model_id, image, confidence)
RoboflowInference-->>HttpClient: JSON response (predictions[])
HttpClient-->>Validator: RoboflowResponse
Validator->>Validator: EvaluatePredictions(response)
Validator->>Validator: apply MinConfidence filter
Validator->>Validator: apply AllowedLabels filter
Validator-->>Validator: Verdict (Allow/Reject)
Validator-->>Client: return Verdict
Client->>Client: short-circuit on first Reject
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The review spans extensive planning documentation (high-density context and verification artifacts) and a complete new plugin implementation. However, the code follows an established CPAI pattern with straightforward HTTP request/response handling, familiar DI wiring, and focused unit tests. The heterogeneity comes from the large file count and mixed documentation/code, but individual pieces are consistent and repetitive in structure. Possibly related issues
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #42 +/- ##
==========================================
+ Coverage 90.42% 90.90% +0.48%
==========================================
Files 51 55 +4
Lines 1472 1595 +123
Branches 237 253 +16
==========================================
+ Hits 1331 1450 +119
- Misses 73 75 +2
- Partials 68 70 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…coverage PR #42 hit codecov/project FAIL because PluginRegistrar.cs was at 9.75% patch coverage and dragged the project number below the 1% drop threshold. The CPAI plugin has the same gap; future PRs can backfill those tests. New tests exercise: 1. One Roboflow entry → keyed IValidationPlugin + named HttpClient + named options resolve 2. Two Roboflow entries (different ModelIds) register independently 3. Non-Roboflow Type discriminator (CodeProjectAi) is skipped — coexistence 4. Missing Validators section returns cleanly without throwing 5. AllowInvalidCertificates=true configures the TLS-bypass branch (registrar:64-69) Pattern mirrors tests/FrigateRelay.Plugins.BlueIris.Tests/BlueIrisActionPluginTests.cs:34 (BuildProviderViaRegistrar) which builds a ServiceProvider via the registrar and asserts DI registrations resolve. Test count: 251 → 256.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
src/FrigateRelay.Plugins.Roboflow/RoboflowResponse.cs (1)
21-28: ⚡ Quick win
Labeldeclared non-nullable but STJ won't enforce it at deserialisation.
RoboflowPrediction.Labelis typed asstring(non-nullable), but System.Text.Json does not enforce nullability on deserialized positional record parameters by default (RespectNullableAnnotationsisfalseunless explicitly opted in). If Roboflow ever omits"class"from a prediction object (e.g., an unknown model variant or a future API change),Labelwill silently benullat runtime, which can causeNullReferenceExceptiondownstream whenEvaluatePredictionsstring-compares it againstAllowedLabels.Consider either marking it nullable (
string?) or adding a null-guard in the evaluation path.🛡️ Option A — defensive nullable annotation
internal sealed record RoboflowPrediction( - [property: JsonPropertyName("class")] string Label, + [property: JsonPropertyName("class")] string? Label, [property: JsonPropertyName("confidence")] double Confidence,Then skip any prediction with a null label in
EvaluatePredictions:// skip predictions with no label if (string.IsNullOrEmpty(p.Label)) continue;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/FrigateRelay.Plugins.Roboflow/RoboflowResponse.cs` around lines 21 - 28, RoboflowPrediction.Label is declared non-nullable but STJ can deserialize a null for the "class" property; update the model or evaluation to avoid NREs: either change the positional parameter type to string? in RoboflowPrediction so missing/nullable JSON maps safely, or leave it non-nullable and add a null/empty check in EvaluatePredictions (e.g., skip any prediction where p.Label is null or empty before comparing against AllowedLabels); reference RoboflowPrediction.Label and the EvaluatePredictions/AllowedLabels logic when making the change.tests/FrigateRelay.Plugins.Roboflow.Tests/RoboflowValidatorTests.cs (1)
250-252: ⚡ Quick winMatch on the request body, not just the route.
StubInferEndpointaccepts any POST payload, so this suite will still pass if the serializer regresses frommodel_idtomodelId, drops the base64 envelope, or stops sendingconfidence. Adding a body matcher for the load-bearing fields would make these tests protect the live Roboflow contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/FrigateRelay.Plugins.Roboflow.Tests/RoboflowValidatorTests.cs` around lines 250 - 252, Update StubInferEndpoint to assert the POST body contains the expected Roboflow contract fields rather than accepting any payload: when configuring the WireMockServer (ws) for the "/infer/object_detection" POST in StubInferEndpoint, add a request body matcher that verifies required keys and shapes (e.g., "model_id" exact key name, the base64-wrapped image payload field, and "confidence" threshold field) and only respondWith the given JSON when that matcher passes; reference the StubInferEndpoint method and the Request.Create().WithPath("/infer/object_detection").UsingPost() chain to add the body validation so tests will fail if the serializer or payload shape regresses.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.shipyard/phases/14/plans/PLAN-2.2.md:
- Line 54: The PLAn spec for DOODS2's Usings.cs is missing the global using for
FluentAssertions, so add `global using FluentAssertions;` to the Usings.cs spec
referenced in PLAN-2.2 so test projects get the FluentAssertions extension
methods; update the description on line 54 (the Usings.cs entry) to include this
third global using alongside `global using FrigateRelay.TestHelpers;` and
`global using Microsoft.VisualStudio.TestTools.UnitTesting;` so Task 1 produces
compilable tests that can use Should() chains without adding local usings.
In @.shipyard/phases/14/plans/PLAN-3.1.md:
- Around line 62-63: The documentation uses the wrong verdict term
"Verdict.Allow"; update the wording to the project's actual API naming (e.g.,
use Verdict.Pass() for success and Verdict.Fail(...) for failure) so the text
consistently reflects the contract used elsewhere (replace "Verdict.Allow" with
"Verdict.Pass()" and ensure surrounding explanation matches the Pass/Fail
semantics).
In @.shipyard/phases/14/plans/PLAN-3.2.md:
- Around line 30-31: The plan contains a contradictory cancellation contract:
one part requires host-shutdown cancellation to propagate (validators throwing
OperationCanceledException when ct.IsCancellationRequested must bubble out of
Task.WhenAll and unwind the consumer loop) while another part asserts no
exception should escape; pick a single invariant and make the spec consistent by
either (A) explicitly stating that OperationCanceledException is allowed to
propagate from validators and that Task.WhenAll should rethrow to unwind the
consumer loop (reference Task.WhenAll, OperationCanceledException,
ct.IsCancellationRequested, consumer loop), or (B) stating that host-shutdown
must be swallowed and the consumer loop must handle/capture cancellation so no
exception bubbles out; update the statements at both the earlier clause
(currently mentioning Task.WhenAll) and the later assertion (lines ~142–143) to
match the chosen behavior and adjust any test expectations accordingly.
In @.shipyard/phases/14/plans/PLAN-3.3.md:
- Around line 66-67: Replace the brittle fixed 100ms delta check with a
deterministic overlap proof: modify the test that inspects WireMock
RequestMessage.DateTime to either (a) inject a known per-validator artificial
delay and assert the overall elapsed time is less than the sum of sequential
timeouts (cpai_timeout + roboflow_timeout) to prove concurrency, or (b) compute
a looser threshold based on cpai_timeout and roboflow_timeout plus a small
safety margin and assert the two RequestMessage.DateTime values overlap within
that bound; update the assertion logic to use these computed values instead of
the hardcoded 100ms.
In @.shipyard/phases/14/RESEARCH.md:
- Around line 87-88: Update the test guidance to standardize on the approved
CapturingLogger<T> pattern: in the description for
CodeProjectAiValidatorTests.cs remove/replace any mention of using NSubstitute
for ILogger and state that tests should use the shared FrigateRelay.TestHelpers
CapturingLogger<T> for all log assertions; ensure the sentence references the
test file name (CodeProjectAiValidatorTests.cs) and the helper type
(CapturingLogger<T>) so implementers know which logger pattern to use
consistently.
In @.shipyard/ROADMAP.md:
- Around line 441-442: Update the ROADMAP.md Phase 14 deliverable to reference
the correct owning namespace/assembly for ActionEntry: change the incorrect
"FrigateRelay.Abstractions" mention to "FrigateRelay.Host/Configuration" so the
planning text aligns with this PR's implementation; ensure the same correction
is applied to any other Phase 14 lines that mention ActionEntry to avoid
misdirecting implementers, and keep the rest of the paragraph about
ParallelValidators unchanged.
In `@CHANGELOG.md`:
- Around line 23-25: The docker run example pins the image to "latest" which can
drift; update the image tag string
"roboflow/roboflow-inference-server-cpu:latest" to the verified version used in
validation (e.g., "roboflow/roboflow-inference-server-cpu:v1.2.7") so the manual
smoke recipe uses a fixed, known-good image.
In `@src/FrigateRelay.Plugins.Roboflow/RoboflowOptions.cs`:
- Around line 37-73: Add an optional ApiKey property to the RoboflowOptions type
so authenticated Roboflow deployments can be configured: declare public string
ApiKey { get; set; } = ""; include an XML doc comment explaining it is optional
and used for API-key auth, and follow the same default/initialization pattern as
ModelId/AllowedLabels (empty string when not set) so the validator can read it
and attach credentials to requests.
In `@src/FrigateRelay.Plugins.Roboflow/RoboflowValidator.cs`:
- Around line 72-95: The JSON deserialization call
ReadFromJsonAsync<RoboflowResponse> can throw and currently escapes
ValidateAsync; catch the exception(s) thrown by deserialization (e.g.,
JsonException or InvalidOperationException) around the await
response.Content.ReadFromJsonAsync<RoboflowResponse>(ct) call, log via
Log.ValidatorUnavailable/_logger/_name/ctx.EventId as done for
HttpRequestException, and return the appropriate verdict using _opts.OnError
(RoboflowValidatorErrorMode.FailOpen -> Verdict.Pass(), otherwise Verdict.Fail
with a token like "validator_unavailable" or include ex.Message) so
deserialization failures follow the same FailOpen/FailClosed path as other
network errors before calling EvaluatePredictions.
---
Nitpick comments:
In `@src/FrigateRelay.Plugins.Roboflow/RoboflowResponse.cs`:
- Around line 21-28: RoboflowPrediction.Label is declared non-nullable but STJ
can deserialize a null for the "class" property; update the model or evaluation
to avoid NREs: either change the positional parameter type to string? in
RoboflowPrediction so missing/nullable JSON maps safely, or leave it
non-nullable and add a null/empty check in EvaluatePredictions (e.g., skip any
prediction where p.Label is null or empty before comparing against
AllowedLabels); reference RoboflowPrediction.Label and the
EvaluatePredictions/AllowedLabels logic when making the change.
In `@tests/FrigateRelay.Plugins.Roboflow.Tests/RoboflowValidatorTests.cs`:
- Around line 250-252: Update StubInferEndpoint to assert the POST body contains
the expected Roboflow contract fields rather than accepting any payload: when
configuring the WireMockServer (ws) for the "/infer/object_detection" POST in
StubInferEndpoint, add a request body matcher that verifies required keys and
shapes (e.g., "model_id" exact key name, the base64-wrapped image payload field,
and "confidence" threshold field) and only respondWith the given JSON when that
matcher passes; reference the StubInferEndpoint method and the
Request.Create().WithPath("/infer/object_detection").UsingPost() chain to add
the body validation so tests will fail if the serializer or payload shape
regresses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a50b6f15-a16e-4c97-9389-4bc97cee37d3
📒 Files selected for processing (32)
.shipyard/HISTORY.md.shipyard/PROJECT.md.shipyard/ROADMAP.md.shipyard/STATE.json.shipyard/phases/14/CONTEXT-14.md.shipyard/phases/14/CRITIQUE.md.shipyard/phases/14/RESEARCH.md.shipyard/phases/14/VERIFICATION.md.shipyard/phases/14/plans/PLAN-1.1.md.shipyard/phases/14/plans/PLAN-1.2.md.shipyard/phases/14/plans/PLAN-2.1.md.shipyard/phases/14/plans/PLAN-2.2.md.shipyard/phases/14/plans/PLAN-2.3.md.shipyard/phases/14/plans/PLAN-3.1.md.shipyard/phases/14/plans/PLAN-3.2.md.shipyard/phases/14/plans/PLAN-3.3.md.shipyard/phases/14/results/REVIEW-1.1.md.shipyard/phases/14/results/REVIEW-1.2.md.shipyard/phases/14/results/SUMMARY-1.1.md.shipyard/phases/14/results/SUMMARY-1.2.mdCHANGELOG.mdFrigateRelay.slnsrc/FrigateRelay.Host/FrigateRelay.Host.csprojsrc/FrigateRelay.Host/HostBootstrap.cssrc/FrigateRelay.Plugins.Roboflow/FrigateRelay.Plugins.Roboflow.csprojsrc/FrigateRelay.Plugins.Roboflow/PluginRegistrar.cssrc/FrigateRelay.Plugins.Roboflow/RoboflowOptions.cssrc/FrigateRelay.Plugins.Roboflow/RoboflowResponse.cssrc/FrigateRelay.Plugins.Roboflow/RoboflowValidator.cstests/FrigateRelay.Plugins.Roboflow.Tests/FrigateRelay.Plugins.Roboflow.Tests.csprojtests/FrigateRelay.Plugins.Roboflow.Tests/RoboflowValidatorTests.cstests/FrigateRelay.Plugins.Roboflow.Tests/Usings.cs
Source-code fixes from CodeRabbit's PR #42 review: 1. **MAJOR — RoboflowOptions.cs:** add optional `ApiKey` property for authenticated self-hosted Roboflow Inference deployments. Sent as `api_key` field in the POST body (per Roboflow Inference v1.x ObjectDetectionInferenceRequest schema). Empty default; should be supplied via env var or user-secrets. 2. **MAJOR — RoboflowValidator.cs:** add `catch (JsonException)` block above the existing HttpRequestException catch. Roboflow could return non-JSON (HTML error page from a misbehaving proxy, truncated body) and `ReadFromJsonAsync` would throw — escaping ValidateAsync and bypassing the OnError contract entirely. Now routes through FailOpen/FailClosed identically to network errors. 3. **NITPICK — RoboflowResponse.cs:** mark `RoboflowPrediction.Label` as `string?` since STJ does not enforce non-nullable annotations on positional record params (RespectNullableAnnotations is off by default). EvaluatePredictions skips predictions with a null/empty label so a missing `class` field on the wire can't NRE the AllowedLabels comparison. 4. **NITPICK — RoboflowValidatorTests.cs:** harden `StubInferEndpoint` with `JsonPathMatcher` body matchers for `model_id`, `image.type`, `image.value`, `confidence`. A future serializer regression that drops or renames any of these fields now fails every test using the stub. 5. Added 2 new tests covering the JsonException branch (test 10) and the ApiKey forwarding (test 11). Test count: 251 → 258 (16 in the Roboflow project).
Documentation corrections from CodeRabbit's PR #42 review: - **CHANGELOG.md** — pin manual-smoke recipe image tag from `:latest` to `:1.2.7` (the version verified during validator implementation). Same image digest; prevents drift if upstream pushes a non-compatible :latest in the future. - **ROADMAP.md:441** — `ActionEntry` is host-internal (declared in `src/FrigateRelay.Host/Configuration/ActionEntry.cs` per RESEARCH §2.5), NOT in `FrigateRelay.Abstractions`. Updated the Phase 14 deliverable text so future Wave 3 builders aren't misled about the assembly. - **RESEARCH.md:87** — removed mention of NSubstitute on ILogger; replaced with explicit standardization on `CapturingLogger<T>` from `FrigateRelay.TestHelpers` per CLAUDE.md conventions. NSubstitute on the generic `Log<TState>` is fragile. - **PLAN-2.2.md:54** — DOODS2 `Usings.cs` spec now includes `global using FluentAssertions;` (caught in PR #42 REVIEW-1.2). - **PLAN-3.1.md:62** — Replace `Verdict.Allow` with `Verdict.Pass()` (project's actual API naming). - **PLAN-3.2.md:30 & :142** — Resolve the contradictory cancellation contract by making the chain explicit: validator throws OperationCanceledException → propagates out of Task.WhenAll → propagates up the stack → caught by ConsumeAsync's existing outer `catch (OperationCanceledException) when (ct.IsCancellationRequested)` at ChannelActionDispatcher.cs:259 (CONTEXT-9 D4 / ID-6 fix). Both statements now describe the same flow. - **PLAN-3.3.md:67** — Replace brittle 100ms hardcoded threshold with a computed assertion (per-validator delay × 1.5 safety margin), plus a documented `SemaphoreSlim` fallback for environments where wall-clock timing is too noisy. Build clean (0 warnings); 258/258 tests still passing.
blehnen
added a commit
that referenced
this pull request
May 6, 2026
This was referenced May 6, 2026
blehnen
added a commit
that referenced
this pull request
May 7, 2026
PR #42 (#13 Roboflow), PR #43 (#14 DOODS2), PR #44 (#23 ParallelValidators + StopAsync hardening) all merged to main. 291/291 tests passing across the suite (49 net-new tests for the phase: 242 → 291). CHANGELOG [Unreleased] is fully populated and ready for promotion to [1.2.0] per RELEASING.md. Next step is operator-cut `git tag v1.2.0`, which triggers release.yml's smoke + push-multiarch GHCR pipeline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blehnen
added a commit
that referenced
this pull request
May 7, 2026
Mirrors the Roboflow PluginRegistrar test pattern that closed the same patch-coverage gap on PR #42. The DOODS2 PluginRegistrar was at 9.76% (4/41 lines covered) on PR #43 because the validator unit tests only exercise Doods2Validator directly — they don't touch the DI registration path. Five tests now exercise: - Single-entry registration (keyed validator + named options + named HttpClient). - Two-entry independence (per-instance keyed singletons with distinct DetectorName). - Type-discriminator filtering (DOODS2 registrar skips CodeProjectAi entries). - Missing Validators section (clean return, no throw). - AllowInvalidCertificates=true (lazy SocketsHttpHandler TLS-bypass branch). 5/5 passing in 326 ms. Pushed directly to main per operator authorization (test-only, brings DOODS2 patch coverage above codecov's threshold for the v1.2.0 release window). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
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
1 participant
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.
Summary
FrigateRelay.Plugins.Roboflowvalidator plugin — HTTP-basedIValidationPluginagainst a self-hosted Roboflow Inference server (http://<host>:9001). Closes feat: add Roboflow Inference validator (RF-DETR support) #13.ModelId(e.g.rfdetr-base),MinConfidence,AllowedLabels,OnError(FailClosed/FailOpen),Timeout,AllowInvalidCertificates.IPluginRegistrarenumeratesValidators:<key>config, filters byType: "Roboflow", registers per-instanceHttpClient(with optional TLS bypass) +AddKeyedSingleton<IValidationPlugin>. NoAddResilienceHandler(validator pattern: pre-action gates don't retry per CONTEXT-7 D4).POST /infer/object_detectionwith{model_id, image: {type:"base64", value}, confidence}request,{predictions, image, time}response.Why no Testcontainers integration test
The upstream
roboflow/roboflow-inference-server-cpu:latestimage is ~16 GB — exceeds the GitHub Actions Linux runner's free-disk budget (~14 GB onubuntu-latest). WireMock-driven unit tests cover the validator's behavior contract; CHANGELOG includes a manual-smoke recipe operators can run locally against a real Roboflow Inference container.This was OQ-2 in the Phase 14 plan and is the documented fallback per
.shipyard/phases/14/CONTEXT-14.md.Wave 1 of 3 in Phase 14 / v1.2
This PR is the first of three sequential PRs against
main:ParallelValidatorsopt-in (feat(validators): optional parallel execution with all-pass aggregation #23)CHANGELOG
[Unreleased]accumulates bullets across all three; promotes to[1.2.0]after the third merges.Test plan
dotnet build FrigateRelay.sln -c Release— 0 warnings, 0 errorsbash .github/scripts/run-tests.sh --skip-integration— 251/251 passingGrpc.*,App.Metrics,OpenTracing,Jaeger.*,.Result,.Wait,ServicePointManager(except doc-comments saying "never use it"), no hard-coded IPs in sourcehttp://192.168.0.2:19004once merged + deployed locallyReview trail
.shipyard/phases/14/results/REVIEW-1.1.md— PLAN-1.1 (scaffold + DI) PASS, 0 critical.shipyard/phases/14/results/REVIEW-1.2.md— PLAN-1.2 (tests + CHANGELOG) PASS after one revision cycle addressing 3 Important findings inline (cancellation test now asserts WireMock untouched; FailOpen-on-HTTP-error test added;Usings.csnow hasglobal using FluentAssertions;)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes