docs(diagnostics): command history is oldest first, not newest first (part of #344)

[tylerkron]

Summary

GetCommandHistoryAsync is documented as returning the remembered commands "newest first". The device actually emits them oldest first. This PR fixes the prose — it does not change the returned order.

Not merging — for review.

Bench evidence

Confirmed three times on a real Nq1 running fw 3.7.2. Full evidence is posted at daqifi-core#344 (comment).

The wire format is a Last N commands: header followed by <n>: <command> lines where <n> counts backwards from the present — 1: is the most recent command, and it is printed last:

Last 3 commands:
3: SYSTem:LOG:TEST
2: SYSTem:STReam:STATS?
1: SYSTem:MEMory:FREE?

The decisive evidence: when you call the history query itself, SYSTem:LOG:CMDHistory? appears in the last slot of the returned list. It is necessarily the most recent command the device has seen at the moment it builds the reply, so if the list were newest-first it would be at index 0.

Why the docs, and not the code

The parser and its unit-test fixture were both already correct. CommandHistoryParserTests already used the true descending 3: / 2: / 1: layout, and CommandHistoryParser faithfully preserves the device's order. Only the prose was wrong.

Deliberately not reversing the returned list: the device's order is a meaningful chronological transcript, and flipping it would be a silent breaking change for any consumer already reading it correctly.

Changes

Five sites claimed "newest first" and all five were wrong. Each now says oldest first and explains that the stripped <n>: prefix counts backwards from the present:

1. IDeviceDiagnostics.cs:55 — <returns>, plus a new <remarks> explaining the numbering
2. CommandHistoryParser.cs:12 — class remarks
3. CommandHistoryParser.cs:24 — Parse <returns>
4. ScpiMessageProducer.cs:1186 — GetCommandHistory remarks
5. DEVICE_INTERFACES.md:911 — the diagnostics recipe comment (a fifth site found during the sweep, same wrong claim)

Plus a new parser test, Parse_ReturnsOldestFirst_BecauseDeviceNumbersLinesBackwardsAndPrintsNewestLast, that pins the direction against the real Last 3 commands: / 3: A / 2: B / 1: C layout and asserts exactly [A, B, C] — with C documented as the newest — so the contract cannot silently drift back.

No production behavior changes; the only non-doc change is the added test.

Testing

Build clean on both TFMs, 0 warnings / 0 errors. Full suite run on net9.0 and net10.0:

• Daqifi.Core.Tests — 2699 passed, 2 skipped, 5 failed on each TFM
• Daqifi.Mcp.Tests — 23 passed
• Diagnostics area specifically (including the new test) — 59/59 passed on both TFMs

⚠️ The 5 failures are pre-existing on main and unrelated to this PR. All five are in FirmwareUpdateServiceTests (WiFi module update / LAN recovery sequencing) and appear to come from the recently merged #444 / #445 WiFi prep commits. Verified by stashing this branch's changes and re-running against a clean tree at 7965b26 — the identical 5 tests fail:

• UpdateWifiModuleAsync_WhenCanceledAfterEnteringUpdateMode_StillTakesDeviceBackOut
• UpdateWifiModuleAsync_WhenCanceledDuringTransparentModeExitSettle_LeavesLanRestoreUnsent
• UpdateWifiModuleAsync_WhenFlashToolFails_TakesDeviceBackOutOfLanUpdateMode
• UpdateWifiModuleAsync_WhenRecoveryBudgetExpiresAfterReconnect_StillFinishesTheBridgeExit
• UpdateWifiModuleAsync_WhenRecoveryBudgetExpiresWaitingForReconnect_SendsNoBridgeExit

They all assert on expected SCPI command sequences and are failing on an unexpected leading SYSTem:POWer:STATe 1. Worth a separate look — this PR touches none of that code.

Part of #344.

🤖 Generated with Claude Code

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Fix diagnostics docs: SCPI command history is oldest-first (add test)

📝 Documentation 🧪 Tests 🕐 10-20 Minutes

AI Description

• Correct diagnostics docs to state command history is returned oldest-first.
• Explain device line numbering: "1:" is newest, printed last.
• Add a unit test to lock the ordering contract against regressions.

Diagram

graph TD
  docs["DEVICE_INTERFACES.md"] --> iface["IDeviceDiagnostics.GetCommandHistoryAsync"] --> prod["ScpiMessageProducer.GetCommandHistory"] --> dev{{"Nq1 device"}} --> parser["CommandHistoryParser.Parse"] --> out["History list (oldest-first)"]
  tests["CommandHistoryParserTests"] --> parser

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Reverse returned list to 'newest-first'

• ➕ Matches the previously (incorrectly) documented contract
• ➕ Potentially more convenient for callers who want recent items first
• ➖ Silent breaking change for any consumer already reading the chronological order correctly
• ➖ Would fight the device’s natural transcript ordering and the established parser behavior

2. Expose both orders explicitly (e.g., option/overload)

• ➕ Gives callers a choice without breaking existing behavior
• ➕ Makes intent explicit in the API surface
• ➖ API surface expansion for a behavior that is already correct
• ➖ Still requires deciding and documenting a default order

Recommendation: Keep behavior as-is and correct the documentation (as this PR does), with a regression test to prevent contract drift. Reversing the list would be a risky breaking change given consumers may already rely on the device’s chronological transcript ordering.

Files changed (5) +41 / -8

Tests (1) +22 / -0

CommandHistoryParserTests.csAdd regression test asserting oldest-first parsing +22/-0

Add regression test asserting oldest-first parsing

• Adds a unit test that encodes the real device layout (e.g., '3: A', '2: B', '1: C') and asserts the parsed output preserves that order. The test explicitly documents that the newest command is printed last and should not be reversed.

src/Daqifi.Core.Tests/Device/Diagnostics/CommandHistoryParserTests.cs

Documentation (4) +19 / -8

DEVICE_INTERFACES.mdFix example comment: history is oldest-first and numbered backwards +2/-1

Fix example comment: history is oldest-first and numbered backwards

• Updates the diagnostics example comment to state the command history list is oldest-first. Adds a brief explanation that the device numbers lines backwards so the newest command appears last.

docs/DEVICE_INTERFACES.md

ScpiMessageProducer.csCorrect remarks for GetCommandHistory response ordering +4/-2

Correct remarks for GetCommandHistory response ordering

• Updates XML remarks to state returned history is oldest-first and explains the '<n>:' prefix counts backwards from the present. Clarifies that the newest entry is printed last by the firmware.

src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs

CommandHistoryParser.csClarify parser contract: preserves device order (oldest-first) +6/-4

Clarify parser contract: preserves device order (oldest-first)

• Rewrites class remarks to explain the firmware’s numbering and print order and that the parser preserves device order. Updates the Parse() return documentation from newest-first to oldest-first.

src/Daqifi.Core/Device/Diagnostics/CommandHistoryParser.cs

IDeviceDiagnostics.csCorrect GetCommandHistoryAsync docs and add explanatory remarks +7/-1

Correct GetCommandHistoryAsync docs and add explanatory remarks

• Updates the interface contract to say the returned list is oldest-first and adds remarks explaining the device’s backwards numbering and why the newest command appears last. Emphasizes that the numeric prefix is stripped while the device order is preserved.

src/Daqifi.Core/Device/Diagnostics/IDeviceDiagnostics.cs

[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

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