chore(api): check in the public API surface so a break is a reviewable diff

[tylerkron]

What was wrong

ADR 0002 promises that Daqifi.Core keeps source compatibility for its public API — 231 public types, ~2,700 members. Nothing in the build checked that. Delete a public method, narrow a parameter type, add a member to a public interface, and you get a green build and a green CI run. The break shows up later, when daqifi-desktop, Daqifi.Mcp or an outside integrator next restores the package. That is how #557 was found — by an adversarial audit on a PR, not by tooling — and it is the reason #612 and #616 are parked: everyone agrees their fix "breaks the API", but there was no artifact saying what the API even is.

How it was fixed

The public surface is now checked in, as PublicAPI.Shipped.txt (what v1.7.0 published) and PublicAPI.Unshipped.txt (what has been added since), and Microsoft.CodeAnalysis.PublicApiAnalyzers fails the build whenever the code and those files disagree. Changing the API is still allowed — it just has to arrive as an explicit diff to the API file in the same PR, where a reviewer can see it.

Seeding the files against the v1.7.0 tag rather than against main also answered a question nobody could answer before: 51 public members have been added since the release and none removed. The promise has held in practice.

Two things worth pushing back on, so they are not buried:

• Daqifi.Core.csproj is +9 lines; PublicAPI.Shipped.txt is 2,669. That file is a generated baseline and is meant to be skimmed, not read. The reviewable part is the csproj, the .editorconfig stanza, CONTRIBUTING.md, and PublicAPI.Unshipped.txt — 51 lines that should look exactly like the public API added since v1.7.0.
• EnablePackageValidation (the second half of chore(api): nothing tracks the public API surface, but ADR 0002 makes source compatibility a promise #636) is deliberately not here, so this does not close the issue. It runs at dotnet pack time, and CI never packs — only the release workflow does — so it would catch breaks at release rather than in the PR. It also trips CP0003 immediately, because the repo has no <Version> and so packs locally as 1.0.0, which is "lower than" the 1.7.0 baseline. That wants its own decision about repo versioning, not a suppression tacked onto this PR.

Detail

• The protoc-generated DaqifiOutMessage.cs is skipped by the analyzer's own code fix, so its 191 entries were added by hand from the RS0016 diagnostics. Noted in CONTRIBUTING.md for whoever regenerates the protos next. (Its types sit in the global namespace, which is its own small surprise.)
• RS0041 (public members should not use nullable-oblivious types) is switched off for that one generated file. It fires 166 times there and the fix — annotating protoc's output — is not ours to make. The obliviousness is not lost: those entries carry the analyzer's ~ prefix, so a change in their nullability still shows as an API diff. The rule stays active for every hand-written public member.
• RS0026/RS0027 (overloads with optional parameters) do not fire on this baseline, but they will on a future addition that follows the repo's existing optional-parameter convention. Left at default rather than pre-suppressed; that is a decision for the PR that first hits it.

Verified

• Full dotnet test Daqifi.Core.sln -warnaserror green on both frameworks: net9.0 4022 passed / 2 skipped, net10.0 4022 passed / 2 skipped (+7 new cases).
• The guard was mutation-tested three ways, each confirmed to fail the build: add a public member (RS0016), delete an API entry (RS0016), leave a stale entry behind (RS0017).
• The five new tests in PublicApiTrackingTests guard the analyzer's wiring — delete the two AdditionalFiles lines and the analyzer silently finds nothing to compare against and reports nothing at all. Each was checked to fail on a green build; two candidate tests were dropped for failing that check (duplicates are already RS0025, and dropping PrivateAssets already breaks the build).
• No production source changed. Packed from main and from this branch in the same directory: identical package contents, byte-identical XML docs, identical assembly sizes, and no new package dependency.
• Bench, Nq1 fw 3.7.2 on /dev/cu.usbmodem1101: connect → status → AI0/AI1 → 100 Hz → 3 s CSV capture → clean disconnect, exit 0. Non-destructive.

Addresses #636 (leaves the EnablePackageValidation half open, see above).

Not merging — for review.

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Track and enforce the Daqifi.Core public API surface

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Check in v1.7.0 and post-release public API baselines for explicit compatibility review.
• Fail builds when Daqifi.Core’s public surface drifts from tracked signatures.
• Guard analyzer wiring and document generated-code exceptions and release maintenance.

Diagram

graph TD
  Tests["Wiring tests"] --> Build["Core build"] --> Analyzer["API analyzer"] --> Match{"Surface matches?"}
  Shipped["Shipped baseline"] --> Analyzer
  Unshipped["Unshipped additions"] --> Analyzer
  Match -->|Yes| Pass["Build passes"]
  Match -->|No| Fail["Build fails"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Package validation at pack time

• ➕ Compares produced packages against a released package
• ➕ Provides binary compatibility diagnostics
• ➖ CI does not currently run dotnet pack
• ➖ Repository versioning currently triggers CP0003
• ➖ Detects breaks later than normal builds

2. Custom reflection snapshot tooling

• ➕ Could tailor signature formatting and policy
• ➕ Avoids an analyzer dependency
• ➖ Requires maintaining compatibility logic and code generation
• ➖ More likely to miss C# API details such as nullability and generated members

Recommendation: Use the checked-in PublicApiAnalyzers baselines as implemented because they fail during ordinary builds and make every API change reviewable with standard Roslyn semantics. Consider package validation separately after CI packing and repository versioning are resolved; it complements rather than replaces this source-compatibility guard.

Files changed (6) +2948 / -0

Tests (1) +173 / -0

PublicApiTrackingTests.csProtect public API analyzer wiring and baseline integrity +173/-0

Protect public API analyzer wiring and baseline integrity

• Adds tests for the analyzer package, AdditionalFiles declarations, baseline existence, and nullability directives. Reflection-based checks independently ensure every exported type appears in a baseline and validate generic, nested, and global-namespace naming assumptions.

src/Daqifi.Core.Tests/Build/PublicApiTrackingTests.cs

Documentation (1) +29 / -0

CONTRIBUTING.mdDocument the public API change workflow +29/-0

Document the public API change workflow

• Explains shipped versus unshipped baselines, analyzer remediation, release rollover, and deliberate removal handling. It also documents the manual process required for generated protobuf signatures.

CONTRIBUTING.md

Other (4) +2746 / -0

.editorconfigScope nullable API diagnostics around generated protobuf code +16/-0

Scope nullable API diagnostics around generated protobuf code

• Disables RS0041 only for the protoc-generated DaqifiOutMessage.cs file. The checked-in baseline still records its nullable-oblivious signatures, while handwritten public APIs remain guarded.

.editorconfig

Daqifi.Core.csprojEnable Roslyn public API compatibility enforcement +9/-0

Enable Roslyn public API compatibility enforcement

• Adds Microsoft.CodeAnalysis.PublicApiAnalyzers as a private build dependency and supplies both API baseline files as analyzer inputs. RS0016 and RS0017 can now fail normal builds when source signatures drift.

src/Daqifi.Core/Daqifi.Core.csproj

PublicAPI.Shipped.txtRecord the v1.7.0 public API baseline +2669/-0

Record the v1.7.0 public API baseline

• Checks in the nullable-aware signature inventory for the public surface published by v1.7.0, including generated global-namespace protobuf members and nullable-oblivious markers.

src/Daqifi.Core/PublicAPI.Shipped.txt

PublicAPI.Unshipped.txtRecord public API additions since v1.7.0 +52/-0

Record public API additions since v1.7.0

• Tracks the 51 members added after v1.7.0, primarily live and SD-card CSV export APIs. This separates pending additions from the shipped compatibility baseline.

src/Daqifi.Core/PublicAPI.Unshipped.txt

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Fallback guard misses API drift ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

EveryPublicTypeInTheAssembly_IsDeclaredInAnApiFile only checks that each currently exported type
name occurs in either file, so it still passes when a public member/signature changes or a public
type/member is removed while the analyzer is suppressed. This defeats the test's stated purpose of
independently detecting stale API files when analyzer enforcement is disabled.

Code

src/Daqifi.Core.Tests/Build/PublicApiTrackingTests.cs[R114-117]

+        var missing = typeof(DaqifiDevice).Assembly
+            .GetExportedTypes()
+            .Select(ApiNameOf)
+            .Where(name => !declared.Contains(name))

Relevance

●●● Strong

Recent history accepts strengthening tests to detect omitted public behavior; this directly matches
the PR’s independent API-drift goal.

PR-#647
PR-#660
PR-#569

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test comments explicitly claim an independent stale-file check if analyzer enforcement is
disabled, while its implementation enumerates only Assembly.GetExportedTypes() and compares
generated type-name strings against the files. It neither reflects members/signatures nor checks for
API-file entries absent from the assembly, even though the project wiring applies RS0016/RS0017 to
the complete public surface.

src/Daqifi.Core.Tests/Build/PublicApiTrackingTests.cs[21-30]
src/Daqifi.Core.Tests/Build/PublicApiTrackingTests.cs[104-125]
src/Daqifi.Core/Daqifi.Core.csproj[31-39]

Agent prompt

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

## Issue description
The fallback test compares only exported type names, so it cannot detect changed or removed members, changed signatures/nullability, or removed public types when analyzer diagnostics are disabled.

## Issue Context
The test documentation says this reflection check protects against stale API files if PublicApiAnalyzers is suppressed, but `GetExportedTypes().Select(ApiNameOf)` only verifies one direction for type names. Implement a complete surface comparison in both directions, or narrow the test comments/name so they do not claim unsupported protection.

## Fix Focus Areas
- src/Daqifi.Core.Tests/Build/PublicApiTrackingTests.cs[104-125]
- src/Daqifi.Core.Tests/Build/PublicApiTrackingTests.cs[21-30]

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

---

Context sources

Review mode: ⏭️ Skipped: The latest push only revises explanatory comments in an existing test file; it introduces no executable or behavioral change.

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 9f0a5d4

[tylerkron]

Qodo-clean, CI green — ready for review.

Head 9f0a5d4: 2 Qodo rounds (round 1 found one valid finding — the reflection fallback test's comments overclaimed what it covers; round 2 clean, re-checked after settling), 0 unresolved threads, all four CI jobs green. The one CI failure in between was DaqifiDeviceStaleTextLineTests.ExecuteTextCommand_DoesNotExitEarlyWhenOnlyAStaleBlankPrecedesTheRealResponse on macOS/net9 — a timing flake unrelated to this branch (the round-2 commit changed comments only); it passed on rerun with no code change.