fix(mcp): reject over-max sample rates instead of clamping by tylerkron · Pull Request #412 · daqifi/daqifi-core

tylerkron · 2026-07-30T19:35:58Z

Summary

Closes #410.

DaqifiStreamingDevice.StreamingFrequency's setter already throws ArgumentOutOfRangeException when a requested rate exceeds the device's hardware ceiling, and firmware ≥ #524 rejects an over-max rate outright (SCPI -222, no streaming started). Daqifi.Mcp.DaqifiAgent.SetSampleRateAsync was the odd one out: it silently clamped to the effective cap and reported the adjustment in the result, so the same "rate too high" input produced opposite outcomes depending on which entry point a caller used.

This standardizes on throw: SetSampleRateAsync now rejects a request above the effective cap (device hardware max, or a lower --max-sample-rate-hz) with a clear InvalidOperationException, which the MCP tool layer's existing Guard/GuardAsync wrapper already surfaces to the calling agent as a tool-call error with that message — no new plumbing needed.

SampleRateResult shrinks from (DeviceId, RequestedRateHz, AppliedRateHz, Clamped, Note) to (DeviceId, RequestedRateHz) since there's no longer an "applied but different" case to report. This is a breaking change to the set_sample_rate MCP tool's JSON output for any existing client.
ScpiMessageProducer remains intentionally unvalidating (wire-format layer; already correctly documented per feat(device): read the live capability document and merge it into DeviceCapabilities (closes #390) #404) — untouched by this PR.

Test plan

dotnet test Daqifi.Core.sln — 2185 Core tests + 23 Mcp tests pass.
Bench-tested against a real Nq1 device (/dev/cu.usbmodem1101): built the Core example CLI against this worktree's local Daqifi.Core, ran a 3s smoke stream on channels 0+1 at 10 Hz — clean connect/stream/disconnect, exit 0. (This change is Daqifi.Mcp-only, not wire-format, so hardware doesn't exercise the throw path directly; that's covered by the unit tests.)

🤖 Generated with Claude Code

Core's StreamingFrequency setter already throws on an out-of-range rate; Daqifi.Mcp.SetSampleRateAsync silently clamped instead, so the same input produced opposite outcomes depending on which layer a caller went through. Standardize on throw so a client can tell its request was refused rather than silently honored at a lower rate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

tylerkron · 2026-07-30T19:36:01Z

/agentic_review

qodo-code-review · 2026-07-30T19:36:58Z

PR Summary by Qodo

MCP set_sample_rate: reject over-cap sample rates (no clamping)

🐞 Bug fix 📝 Documentation 🕐 20-40 Minutes

AI Description

• Make set_sample_rate throw when requested Hz exceeds the effective device/CLI cap.
• Simplify SampleRateResult payload since “applied but different” is no longer possible.
• Update CLI/tool documentation to describe rejection semantics for --max-sample-rate-hz.

Diagram

graph TD
  A["MCP client"] --> B["set_sample_rate tool"] --> C["DaqifiAgent.SetSampleRateAsync"] --> D["Compute effective cap"] --> E{"rate > cap?"}
  D --> O["ServerOptions: MaxSampleRateHz"]
  D --> M["Device metadata max"]
  E -- "yes" --> F["Throw InvalidOperationException"]
  E -- "no" --> G["Set StreamingFrequency"] --> H["Return SampleRateResult"]

  subgraph Legend
    direction LR
    _ext["External caller"] ~~~ _proc["Processing step"] ~~~ _dec{"Decision"}
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep clamping but return applied rate (status field)

➕ Non-breaking JSON output for existing clients relying on AppliedRateHz/Clamped/Note.
➕ More forgiving UX for interactive callers who “just want streaming to start”.
➖ Preserves inconsistent behavior vs Core and newer firmware (silent partial success).
➖ Harder for clients to detect misconfiguration (they must inspect response fields).

2. Version the tool output schema (v1 clamp, v2 reject)

➕ Avoids breaking existing clients while enabling the stricter semantics.
➕ Provides a clean migration story if MCP clients are already deployed.
➖ More surface area (two tools or version negotiation).
➖ Extra maintenance and documentation burden.

Recommendation: The PR’s approach (reject over-cap and throw) is the most consistent with Core’s StreamingFrequency validation and firmware behavior, and it makes “rate too high” unambiguous to callers. If backward compatibility becomes a concern, schema versioning (or adding deprecated optional fields back) would be the next best step, but the current change is strategically sound for correctness and consistency.

Files changed (5) +17 / -15

Bug fix (2) +12 / -11

DaqifiAgent.csReject over-cap sample rates in SetSampleRateAsync +8/-7

Reject over-cap sample rates in SetSampleRateAsync

• Replaces clamping logic with an explicit check against the computed effective cap (hardware max and optional --max-sample-rate-hz). Throws InvalidOperationException when the requested rate exceeds the cap; otherwise applies the requested rate and returns the simplified result.

src/Daqifi.Mcp/DaqifiAgent.cs

Dtos.csSimplify SampleRateResult DTO and update semantics docs +4/-4

Simplify SampleRateResult DTO and update semantics docs

• Updates the SampleRateResult record to only include DeviceId and RequestedRateHz now that clamping is removed. Adjusts XML docs to state that over-cap requests are rejected rather than adjusted.

src/Daqifi.Mcp/Dtos.cs

Documentation (2) +2 / -2

README.mdDocument --max-sample-rate-hz as rejection, not clamping +1/-1

Document --max-sample-rate-hz as rejection, not clamping
• Updates the CLI help text in the README to reflect that requests above the configured max are rejected instead of clamped.
src/Daqifi.Mcp/README.md

DaqifiTools.csUpdate set_sample_rate tool description to reflect throw-on-overcap +1/-1

Update set_sample_rate tool description to reflect throw-on-overcap
• Rewrites the MCP tool Description attribute to state that requests above the hardware max or --max-sample-rate-hz are rejected (throw) rather than silently clamped.
src/Daqifi.Mcp/Tools/DaqifiTools.cs

Other (1) +3 / -2

ServerOptions.csClarify MaxSampleRateHz behavior as an enforced cap +3/-2

Clarify MaxSampleRateHz behavior as an enforced cap
• Updates option documentation and parsing comments to describe the max as a rejection cap (and why non-positive values are ignored). No parsing behavior change beyond wording.
src/Daqifi.Mcp/ServerOptions.cs

qodo-code-review · 2026-07-30T19:38:44Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Refresh runs while streaming~~ ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New

Description

ConfigureAnalogChannelsAsync/ConfigureDigitalChannelsAsync now refresh the capability document
unconditionally, but ReadCapabilityDocumentAsync explicitly runs a text-mode exchange that pauses
the protobuf consumer and must not be called while streaming. If SD logging is active (it sets
IsStreaming=true), these calls can disrupt/deserialize the stream while still returning success
because refresh failures are swallowed.

Code

src/Daqifi.Mcp/DaqifiAgent.cs[R168-172]

+            // The device's authoritative rate cap (CapabilityStreaming.CurrentMaximumRateHz) is
+            // scoped to the channel set enabled when the document was read — refresh it now so
+            // set_sample_rate validates against the configuration that is actually live.
+            await RefreshCapabilityDocumentAsync(device).ConfigureAwait(false);
+

Relevance

●● Moderate
Streaming/text exchanges are touchy, but no close precedent banning capability refresh during SD
logging/streaming.
PR-#406

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The MCP code refreshes the capability document immediately after channel enable/disable; Core
explicitly documents that capability reads pause the protobuf consumer and must not run while
streaming, and SD logging sets IsStreaming=true, making the new refresh path unsafe during active
logging/streaming.
src/Daqifi.Mcp/DaqifiAgent.cs[168-172]
src/Daqifi.Mcp/DaqifiAgent.cs[216-219]
src/Daqifi.Mcp/DaqifiAgent.cs[492-497]
src/Daqifi.Core/Device/DaqifiDevice.cs[1276-1279]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[2142-2146]
src/Daqifi.Core/Device/IStreamingDevice.cs[9-20]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DaqifiAgent` now calls `RefreshCapabilityDocumentAsync` after channel configuration. That helper calls `device.ReadCapabilityDocumentAsync()`, whose Core docs state it pauses the protobuf consumer and must not be called while streaming. This can break SD logging sessions (which set `IsStreaming=true`) or any active streaming.

### Issue Context
- `StartSdCardLoggingSessionAsync` sets `IsStreaming = true`.
- `ReadCapabilityDocumentAsync` warns: text-mode exchange pauses protobuf consumer; do not call while streaming.
- MCP configure methods do not check `streaming.IsStreaming` before refreshing.

### Fix Focus Areas
- src/Daqifi.Mcp/DaqifiAgent.cs[168-172]
- src/Daqifi.Mcp/DaqifiAgent.cs[216-219]
- src/Daqifi.Mcp/DaqifiAgent.cs[492-497]

### Suggested change
- Add a guard before refresh (or inside `RefreshCapabilityDocumentAsync`) to avoid calling `ReadCapabilityDocumentAsync` when `device is IStreamingDevice { IsStreaming: true }`.
- Decide policy:
 - safest: skip refresh while streaming/logging (leave cap stale until next non-streaming moment), or
 - fail fast: throw an `InvalidOperationException` instructing the caller to stop logging/streaming before reconfiguring.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Cap exceeds absolute maximum~~ ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New

Description

SetSampleRateAsync uses CurrentMaximumRateHz as the effective cap without bounding it to the
absolute ceiling enforced by StreamingFrequency (Capabilities.MaxSamplingRate). If a device reports
an inconsistent capability document where CurrentMaximumRateHz is higher than MaxSamplingRate, the
MCP-layer check can pass but the subsequent StreamingFrequency assignment will throw a different
exception than the intended over-cap InvalidOperationException.

Code

src/Daqifi.Mcp/DaqifiAgent.cs[R363-364]

+            var deviceCap = device.Metadata.CapabilityDocument?.Streaming?.CurrentMaximumRateHz ?? hardwareMax;
+            var cap = _options.MaxSampleRateHz is { } max ? Math.Min(max, deviceCap) : deviceCap;

Relevance

●●● Strong
Team previously accepted clamping sample-rate caps to hardware maxima to avoid inconsistent cap
behavior/exceptions.
PR-#277

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new cap path is based on CurrentMaximumRateHz, which is parsed as a separate field and may exist
without updating MaxSamplingRate, while StreamingFrequency still enforces MaxSamplingRate; this
mismatch can let SetSampleRateAsync’s pre-check succeed but fail at assignment time.
src/Daqifi.Mcp/DaqifiAgent.cs[355-374]
src/Daqifi.Core/Device/Capabilities/CapabilityDocumentParser.cs[290-316]
src/Daqifi.Core/Device/Capabilities/CapabilityDocument.cs[158-167]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[167-182]
PR-#277

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SetSampleRateAsync` validates requested rate against `CurrentMaximumRateHz` but does not ensure this cap is <= the absolute ceiling used by `IStreamingDevice.StreamingFrequency` validation (`Metadata.Capabilities.MaxSamplingRate`). In inconsistent capability-document cases, callers can see unexpected exception types/messages.

### Issue Context
- `CapabilityDocumentParser` reads `current_max_rate_hz` independently of `sample_rate_range_hz.max`.
- `CapabilityDocument.MergeInto` only updates `Capabilities.MaxSamplingRate` when `MaximumSampleRateHz > 0`.
- `StreamingFrequency` setter enforces `value <= Math.Max(1, Metadata.Capabilities.MaxSamplingRate)`.

### Fix Focus Areas
- src/Daqifi.Mcp/DaqifiAgent.cs[355-364]

### Suggested change
- After computing `hardwareMax`, sanitize `deviceCap` defensively:
 - preserve `0` ("no channels enabled") as-is,
 - treat negative as absent (fallback to `hardwareMax`),
 - for positive values, clamp to `hardwareMax` (e.g., `deviceCap = Math.Min(deviceCap, hardwareMax)`).
- Then apply `--max-sample-rate-hz` as a further min.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Silent refresh failures~~ ✓ Resolved 🐞 Bug ◔ Observability ⭐ New

Description

RefreshCapabilityDocumentAsync swallows all exceptions (except cancellation) without logging, so
capability refresh timeouts/transport failures leave CurrentMaximumRateHz stale with no diagnostic
signal. This makes later set_sample_rate validation failures and cap staleness difficult to debug in
production.

Code

src/Daqifi.Mcp/DaqifiAgent.cs[R502-504]

+        catch (Exception)
+        {
+        }

Relevance

●●● Strong
They tend to add logging/diagnostic context for swallowed exceptions; empty catch harms production
debugging.
PR-#354
PR-#360

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The newly added helper has an empty catch-all, so refresh failures are completely hidden from
operators and callers, despite affecting later sample-rate validation behavior.
src/Daqifi.Mcp/DaqifiAgent.cs[488-505]
src/Daqifi.Mcp/Program.cs[18-23]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`RefreshCapabilityDocumentAsync` catches and discards all exceptions, producing silent capability-refresh failures. Because `SetSampleRateAsync` depends on the refreshed cap, silent failures can cause confusing behavior with no way to distinguish "device doesn’t support" from "refresh failed".

### Issue Context
- The helper is explicitly best-effort, so it should remain non-fatal, but it still needs a diagnostic signal when refresh fails unexpectedly.
- The app is already configured to log to stderr; stdout must remain clean for MCP.

### Fix Focus Areas
- src/Daqifi.Mcp/DaqifiAgent.cs[488-506]

### Suggested change
- Keep best-effort behavior but emit a warning/debug log when refresh fails (e.g., inject `ILogger<DaqifiAgent>` into the agent and log at Debug/Warning), or at least write a single-line message to `Console.Error` with exception type/message.
- Consider narrowing the swallow to expected failures (timeouts, feature-not-supported) and logging unexpected exceptions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~Stale help text behavior~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The server now rejects over-max sample rate requests, but the CLI help text still claims
--max-sample-rate-hz will clamp set_sample_rate requests. Users running daqifi-mcp --help will be
misled about the tool’s behavior.

Code

src/Daqifi.Mcp/DaqifiAgent.cs[R354-358]

+            if (rateHz > cap)
+            {
+                throw new InvalidOperationException(
+                    $"Requested {rateHz} Hz exceeds the maximum {cap} Hz for this device.");
+            }

Relevance

●●● Strong
Team often accepts fixes preventing misleading docs/help text after behavior changes.
PR-#391
PR-#375
PR-#349

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR changes SetSampleRateAsync to throw for rateHz > cap (reject behavior). Program prints
ServerOptions.HelpText on --help, but that help text still says the flag clamps requests, while
the README now documents rejection.
src/Daqifi.Mcp/DaqifiAgent.cs[333-362]
src/Daqifi.Mcp/Program.cs[6-9]
src/Daqifi.Mcp/ServerOptions.cs[45-59]
src/Daqifi.Mcp/README.md[47-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SetSampleRateAsync` now **rejects** requests above the effective cap, but the CLI `--help` text still says it **clamps**. Since `Program` prints `ServerOptions.HelpText` for `--help`, the CLI documentation is now incorrect.

### Issue Context
- Runtime behavior: throws when `rateHz > cap`.
- README and tool description already say “reject”.
- Only `ServerOptions.HelpText` is stale.

### Fix Focus Areas
- src/Daqifi.Mcp/ServerOptions.cs[45-59]

### Proposed fix
Update the `--max-sample-rate-hz` line in `ServerOptions.HelpText` from “Clamp …” to “Reject …” to match the new semantics.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

Previous review results

Review updated until commit 9dbf622

Results up to commit 92a40b7 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Stale help text behavior~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The server now rejects over-max sample rate requests, but the CLI help text still claims
--max-sample-rate-hz will clamp set_sample_rate requests. Users running daqifi-mcp --help will be
misled about the tool’s behavior.

Code

src/Daqifi.Mcp/DaqifiAgent.cs[R354-358]

+            if (rateHz > cap)
+            {
+                throw new InvalidOperationException(
+                    $"Requested {rateHz} Hz exceeds the maximum {cap} Hz for this device.");
+            }

Relevance

●●● Strong
Team often accepts fixes preventing misleading docs/help text after behavior changes.
PR-#391
PR-#375
PR-#349

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR changes SetSampleRateAsync to throw for rateHz > cap (reject behavior). Program prints
ServerOptions.HelpText on --help, but that help text still says the flag clamps requests, while
the README now documents rejection.
src/Daqifi.Mcp/DaqifiAgent.cs[333-362]
src/Daqifi.Mcp/Program.cs[6-9]
src/Daqifi.Mcp/ServerOptions.cs[45-59]
src/Daqifi.Mcp/README.md[47-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SetSampleRateAsync` now **rejects** requests above the effective cap, but the CLI `--help` text still says it **clamps**. Since `Program` prints `ServerOptions.HelpText` for `--help`, the CLI documentation is now incorrect.

### Issue Context
- Runtime behavior: throws when `rateHz > cap`.
- README and tool description already say “reject”.
- Only `ServerOptions.HelpText` is stale.

### Fix Focus Areas
- src/Daqifi.Mcp/ServerOptions.cs[45-59]

### Proposed fix
Update the `--max-sample-rate-hz` line in `ServerOptions.HelpText` from “Clamp …” to “Reject …” to match the new semantics.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

The README and tool description already said "reject"; ServerOptions.HelpText (what --help actually prints) still said "clamp". (Qodo review on #412) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

The over-cap check landed in the previous commit validated against Capabilities.MaxSamplingRate, which is the sampling ISR's absolute hardware envelope, not what the device will actually accept for the channels enabled right now (CapabilityStreaming.CurrentMaximumRateHz). Confirmed on a real Nq1 (fw 3.7.2): ISR ceiling reports 22000 Hz while the real cap for 2 enabled channels was 6924 Hz and for 16 channels was 3518 Hz — a request in that gap (e.g. 5000-10000 Hz) sailed through the earlier check silently and would only have failed later, at stream start, via the firmware's SCPI -222. SetSampleRateAsync now prefers CurrentMaximumRateHz when the capability document has one, falling back to the board-derived ceiling otherwise. ConfigureAnalogChannelsAsync/ConfigureDigitalChannelsAsync re-read the capability document (best-effort) after changing the enabled set, so the cap set_sample_rate sees reflects the live configuration rather than whatever was true at connect time. Also corrected the set_sample_rate tool description and README line, which still claimed a flat 1-1000 Hz range. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

tylerkron · 2026-07-30T19:54:01Z

While bench-testing this PR against a real Nq1 (fw 3.7.2), found that the over-cap check validated against Capabilities.MaxSamplingRate (the absolute sampling-ISR ceiling — 22000 Hz on this unit), not what the device will actually accept for the currently enabled channels (CapabilityStreaming.CurrentMaximumRateHz — 6924 Hz with 2 channels enabled, 3518 Hz with all 16). A request in that gap (e.g. 5000-10000 Hz) sailed through silently and would only fail later, at stream start, via firmware's SCPI -222.

Fixed in fe0d131 rather than filing separately:

SetSampleRateAsync now prefers CurrentMaximumRateHz when available, falling back to the board-derived ceiling.
ConfigureAnalogChannelsAsync/ConfigureDigitalChannelsAsync re-read the capability document (best-effort, non-fatal) after changing the enabled channel set, so the cap stays live.
Corrected the set_sample_rate tool description and README line, which still claimed a flat 1-1000 Hz range — real achievable rates vary a lot with channel count on this hardware.

Re-verified end to end against the real device:

Scenario	Cap used	Result
10000 Hz, 2 channels enabled	6924 Hz	Rejected
10000 Hz, 16 channels enabled	3518 Hz (refreshed after configure_analog_channels)	Rejected
500 Hz, 16 channels enabled	3518 Hz	Accepted

/agentic_review

tylerkron · 2026-07-30T20:15:45Z

/agentic_review

qodo-code-review · 2026-07-30T20:23:54Z

Code review by qodo was updated up to the latest commit fe0d131

…lures Three issues from Qodo review of fe0d131: 1. RefreshCapabilityDocumentAsync ran unconditionally after channel config, but ReadCapabilityDocumentAsync's own docs say not to call it while streaming (it pauses the protobuf consumer) -- SD logging sets IsStreaming too. Now skipped outright when streaming.IsStreaming; the cap just stays at its last-known value until the next quiescent refresh. Verified on real hardware: start_sd_logging -> configure_analog_channels mid-log no longer touches the capability document, and logging survives the reconfigure cleanly. 2. SetSampleRateAsync's deviceCap now bounds CurrentMaximumRateHz to hardwareMax, so a self-inconsistent capability document can't produce a cap above the ceiling StreamingFrequency itself enforces (which would otherwise let the check pass and then throw a different exception type one line down). 3. RefreshCapabilityDocumentAsync's catch-all was silent. DaqifiAgent now takes an optional ILogger<DaqifiAgent> (resolved via DI in Program.cs; defaults to NullLogger for the existing test constructor calls) and logs a warning when a refresh fails for a reason other than cancellation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

tylerkron · 2026-07-30T20:48:25Z

Addressed all 3 findings from the last review (commit 9dbf622):

Refresh runs while streaming — RefreshCapabilityDocumentAsync now skips the capability-document read outright when streaming.IsStreaming is true (SD logging included), rather than pausing the protobuf consumer mid-stream. Verified: started SD logging, reconfigured channels mid-log, logging survived cleanly.
Cap exceeds absolute maximum — deviceCap is now bounded to hardwareMax, so an inconsistent capability document can't let the MCP-layer check pass a rate that then throws a different exception at the StreamingFrequency assignment.
Silent refresh failures — DaqifiAgent now takes an optional ILogger<DaqifiAgent> (DI-resolved in production, NullLogger default keeps the existing test constructor calls working) and logs a warning on non-cancellation refresh failures.

CI green, 23/23 Mcp unit tests pass, re-verified against the real Nq1 bench unit.

/agentic_review

tylerkron requested a review from a team as a code owner July 30, 2026 19:35

qodo-code-review Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread src/Daqifi.Mcp/DaqifiAgent.cs

tylerkron and others added 2 commits July 30, 2026 13:42

fix(mcp): update stale --help text for --max-sample-rate-hz

c057326

The README and tool description already said "reject"; ServerOptions.HelpText (what --help actually prints) still said "clamp". (Qodo review on #412) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

qodo-code-review Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread src/Daqifi.Mcp/DaqifiAgent.cs

Comment thread src/Daqifi.Mcp/DaqifiAgent.cs Outdated

Comment thread src/Daqifi.Mcp/DaqifiAgent.cs Outdated

tylerkron merged commit acc3ebd into main Jul 30, 2026
1 check passed

tylerkron deleted the claude/github-issue-410-699faf branch July 30, 2026 20:50

Navigation Menu

Uh oh!

fix(mcp): reject over-max sample rates instead of clamping - #412

fix(mcp): reject over-max sample rates instead of clamping#412
tylerkron merged 4 commits into
mainfrom
claude/github-issue-410-699faf

tylerkron commented Jul 30, 2026

Uh oh!

tylerkron commented Jul 30, 2026

Uh oh!

qodo-code-review Bot commented Jul 30, 2026

Uh oh!

qodo-code-review Bot commented Jul 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

tylerkron commented Jul 30, 2026

Uh oh!

tylerkron commented Jul 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 30, 2026

Uh oh!

tylerkron commented Jul 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tylerkron commented Jul 30, 2026

Summary

Test plan

Uh oh!

tylerkron commented Jul 30, 2026

Uh oh!

qodo-code-review Bot commented Jul 30, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

Uh oh!

tylerkron commented Jul 30, 2026

Uh oh!

tylerkron commented Jul 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 30, 2026

Uh oh!

tylerkron commented Jul 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-code-review Bot commented Jul 30, 2026 •

edited

Loading