[tylerkron]

What was wrong

• Broken link on nuget.org. The packed Daqifi.Mcp README linked to Daqifi.Core with a repo-relative path (../Daqifi.Core). That only works on GitHub, so the link was dead on the package page. docs: absolute GitHub URLs in packed README #775 fixed only the repo-root README.
• Duplicate MCP tests. DaqifiAgentTests had six per-tool *_InReadOnlyMode_IsBlocked facts and a SetSampleRate_BelowOne_Throws fact. The connected-device contract tests in ToolContractTests.cs repeat nearly all of what they check.
• Misleading csproj note. Daqifi.Mcp.csproj ended with a leftover comment saying ModelContextProtocol and Microsoft.Extensions.Hosting were "added via dotnet add package so the restored versions are pinned to whatever is current." That reads as if the versions float. They don't: both are explicit pins (10.0.11 and 2.2.0) that Dependabot bumps, as in chore: Bump the minor-and-patch group with 7 updates #548 and chore: Bump the minor-and-patch group with 7 updates #772.

What changed

• README: the link is now https://github.com/daqifi/daqifi-core/tree/main/src/Daqifi.Core, which returns 200. This is the same absolute style docs: absolute GitHub URLs in packed README #775 used in the root README. No other relative links are left in the packed README.
• csproj: the comment is removed. Nothing else in the file changes.
• Tests: seven facts deleted. The surviving tests were tightened so nothing the deleted facts checked is lost:
  • The deleted read-only facts called each tool with a device id that was never connected. So besides "refused", they also checked that the read-only refusal comes before the device lookup. If it came after, a read-only caller would be told to call connect_device and only find out after connecting that permission was the real obstacle. MutatingTools_AreRefusedAndSendNothing uses a connected device, so it can't see that ordering. A new theory, ReadOnlyModeContractTests.MutatingTools_AreRefusedBeforeTheDeviceIsLookedUp, checks it for every tool in MutatingTools. SdCardAgentGuardTests.DeleteSdFile_InReadOnlyMode_IsRefusedBeforeAnythingElse already checks the same rule for delete_sd_file.
  • Both read-only theories now share one CallMutatingTool helper. Its fallback arm throws instead of quietly calling set_sample_rate, so a tool added to the list without a matching call can't pass by accident.
  • SetSampleRate_BelowOne_IsRejectedBeforeTheDeviceIsTouched now also checks for the >= 1 message that the deleted SetSampleRate_BelowOne_Throws checked. With that, it covers everything the deleted fact did, plus "nothing sent".
  • DaqifiAgentTests.NewAgent drops its readOnly parameter, which nothing uses any more.

Read-only coverage for the six deleted facts (after this PR and #784)

Tool	Deleted fact	Connected device: refused, nothing sent	Unknown device: read-only refusal wins over "not connected"
set_sample_rate	SetSampleRate_InReadOnlyMode_IsBlocked	MutatingTools_AreRefusedAndSendNothing	MutatingTools_AreRefusedBeforeTheDeviceIsLookedUp
configure_analog_channels	ConfigureAnalogChannels_InReadOnlyMode_IsBlocked	same	same
configure_digital_channels	ConfigureDigitalChannels_InReadOnlyMode_IsBlocked	same	same
set_digital_output	SetDigitalOutput_InReadOnlyMode_IsBlocked	same	same
set_pwm_output	SetPwmOutput_InReadOnlyMode_IsBlocked	same	same
disable_pwm	DisablePwm_InReadOnlyMode_IsBlocked	same	same

All of these are [MemberData(nameof(MutatingTools))] cases in ReadOnlyModeContractTests. #784 only rewrites the MutatingTools summary so it stops claiming the SD tools. Those have their own coverage: SdToolContractTests.LoggingTools_AreRefusedInReadOnlyMode and SdCardAgentGuardTests.DeleteSdFile_InReadOnlyMode_IsRefusedBeforeAnythingElse. #784's list still includes all six tools above. I test-merged this branch with the current heads of #784, #790 and #772, and none of them conflict.

One thing is deliberately not kept. SetSampleRate_BelowOne_Throws also happened to check that the rate check runs before the device lookup. The rate check is the first statement in SetSampleRateAsync, so nothing else is re-added for it.

Out of scope

The Idempotent/Title/Range annotations, the disconnect and stop_sd return shapes, SampleRateAdjustedFromHz, and download progress stay as they are until #750, #761, #765, #770 and #778 settle.

Test plan

• dotnet test src/Daqifi.Mcp.Tests/Daqifi.Mcp.Tests.csproj: 271 passed, 0 failed on both net9.0 and net10.0 (262 before, plus 9 new theory cases)
• CI full suite

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Remove redundant MCP tests and fix packaged README link

🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 Less than 10 minutes

AI Description

• Removes weaker tests superseded by connected-device contract coverage.
• Fixes the Daqifi.Core link rendered in the packaged NuGet README.
• Removes an inaccurate comment claiming pinned dependencies float.

Diagram

graph TD
  A["Mutating Tools"] --> B["Read-Only Contract"] --> C["Connected Harness"] --> D["No Commands Sent"]

Loading

High-Level Assessment

The current approach is preferable: centralized contract tests exercise all mutating tools against a connected device and assert that no commands are sent, making the removed per-method tests both weaker and redundant. Retaining them would increase maintenance without adding meaningful coverage.

Files changed (3) +1 / -59

Tests (1) +0 / -55

DaqifiMcpTests.csRemove superseded agent tests +0/-55

Remove superseded agent tests

• Deletes six weak read-only tests that used unconnected device IDs and one redundant sample-rate validation test. Stronger connected-device contract tests retain coverage while additionally asserting that no device commands are sent.

src/Daqifi.Mcp.Tests/DaqifiMcpTests.cs

Documentation (1) +1 / -1

README.mdFix packaged Daqifi.Core link +1/-1

Fix packaged Daqifi.Core link

• Replaces the repository-relative Daqifi.Core link with an absolute GitHub URL so it resolves from the NuGet package README.

src/Daqifi.Mcp/README.md

Other (1) +0 / -3

Daqifi.Mcp.csprojRemove inaccurate dependency comment +0/-3

Remove inaccurate dependency comment

• Removes a stale comment claiming the hosting and MCP package versions float, while both references remain explicitly pinned.

src/Daqifi.Mcp/Daqifi.Mcp.csproj

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 4779f3e 🚀 Fast

Results up to commit e5b1dd8 🚀 Fast

No changes from previous review

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 4779f3e

[tylerkron]

Qodo-clean, CI green — ready for review