Capture Diagnosis Verification Learning

[FrankRay78]

Why

A /ship road-test of a flaky-test fix (issue #220) went down a wrong path: a branch and test edit were made straight off the issue's confident "servers are probed concurrently" root-cause, without reading the implementation. Three clean-context reviewers caught that GetFastestServerByLatencyAsync is a sequential loop and the flake had already been fixed by #221's SynchronousProgress — so the branch premise was invalid. This captures that lesson so it isn't relearned.

What changes

• Extends feedback_read_source_before_designing.md: a confident root-cause handed to you in an issue/spec/ticket is an assumption to verify against current HEAD, not a fact — and when work starts from an issue, verifying its diagnosis is the first step, before creating a branch or editing.
• Records the Flaky test: progress-reporting assertion depends on parallel completion order #220 incident as the confirming NetPace case (the memory previously noted it had no NetPace-specific incident yet).
• Refreshes the MEMORY.md pointer hook to surface the new diagnosis-verification angle.

Docs-only, under .claude/memory/. No source, test, or behaviour changes.

Related

• Supersedes the mis-diagnosis in Flaky test: progress-reporting assertion depends on parallel completion order #220 (closed as already-fixed by Upgrade NetPace from .NET 8 to .NET 10 (LTS) — drops net8.0 TFM (breaking) #221).

[FrankRay78]

@claude Review this pull request. Analyse the code changes and provide feedback covering:

• Bugs or correctness issues
• Security concerns (including any flagged by static analysis)
• Adherence to the project conventions in CLAUDE.md
• Test coverage — do the tests adequately cover the new behaviour?
• Any spec/test-plan mismatches

[claude]

Claude finished @FrankRay78's task in 2m 2s —— View job

---

Review complete ✅

• Gather context (diff, changed files, CLAUDE.md conventions)
• Review for bugs/correctness
• Review for security concerns
• Check adherence to CLAUDE.md conventions
• Check test coverage
• Check spec/test-plan mismatches

This is a small, well-scoped docs-only change to two files under .claude/memory/. Summary of findings:

Bugs / correctness

• None. I verified the factual claims added to feedback_read_source_before_designing.md against current source rather than taking them on faith:
  • GetFastestServerByLatencyAsync in src/NetPace.Core/Clients/Ookla/OoklaSpeedtest.cs:127-160 is indeed a sequential for loop over servers, not a concurrent probe — confirms the memory's claim.
  • SynchronousProgress exists at src/NetPace.Core.Tests/SynchronousProgress.cs, supporting the "Upgrade NetPace from .NET 8 to .NET 10 (LTS) — drops net8.0 TFM (breaking) #221 already fixed the flake" claim.
  • The [[feedback_docs_no_forward_references]] cross-reference resolves to an existing file, and follows this repo's established convention of linking by filename stem (not the frontmatter name: slug) — consistent with the one prior example of this pattern (feedback_speckit_hooks.md:16).

Security

• None applicable — no source, test, or executable content touched.

CLAUDE.md conventions

• Markdown prose is one line per paragraph (no hard-wrap), consistent with the "don't hard-wrap markdown" rule.
• MEMORY.md stays well under the 200-line index limit.
• No specs/ references introduced.

Test coverage / spec-test mismatches

• N/A — memory/documentation files only, no production code or behaviour change, so TDD and test-plan traceability rules don't apply here.

Minor nitpick (non-blocking)

• The frontmatter description: on feedback_read_source_before_designing.md:3 still reads "Before proposing a fix that depends on how a subsystem behaves, read the source..." and doesn't mention the new "verify a handed-down issue/spec diagnosis" angle, even though the body and the MEMORY.md pointer line were both broadened to cover it. Per the memory-system conventions, that description: field is what's used to decide relevance in future conversations — worth a follow-up tweak so retrieval on "issue diagnosis" style triggers also surfaces this file, though the body content and index pointer already carry the substance.

Nice catch turning the /ship road-test misstep into a durable, source-verified lesson rather than just a one-off fix.