Validate DTOs at the mapper boundary (Track C)

[Quasar-Apps-Agent]

PR #24 gave every DTO parameter a default because Firestore's reflective object mapper needs a public no-arg constructor — without it every read threw and the Firebase path was a write-only app. TECH_LEAD_REVIEW §9.4 called that fix "cheapest, but it makes every field silently optional". It was right. This is the follow-through.

The problem the defaults created

With everything defaulted, a partial or malformed document no longer fails at deserialization — it produces a DTO full of "", 0 and emptyList(). The mappers then did one of two unhelpful things:

• Threw something undiagnosable. No enum constant com.example.diamonds.domain.model.VerificationStatus. — loud, but naming neither the document, the field, nor the DTO, and swallowed into a generic Result.Error by firestoreCall.
• Silently substituted a default. An unparseable ReviewDirection became CLIENT_REVIEWS_PROVIDER, filing a cleaner's review of a customer against the customer's own profile. Unrecognised specializations were dropped by mapNotNull, so a provider listing five showed three and nothing said so. An unreadable cleaningType became null, indistinguishable from "unspecified".

The fix

All 13 *Dto.toDomain() mappers now validate, via two helpers:

• enumField / enumFieldOrNull — parse or throw, naming the DTO, the field, the value received and the valid set. Enums are never defaulted on failure: an unrecognised value means the backend sent something this build doesn't understand, and guessing misrepresents the record. The nullable variant keeps absent-as-null while still rejecting a present-but-unparseable value; those two used to collapse together.
• requiredId — rejects a blank identity field. Not a harmless partial read: repositories upsert by primary key, so every malformed record of a type collides on the same "" row and overwrites the last one.

Failures raise MalformedDtoException, added to :core beside OfflineException.

Scope is deliberately the DTO boundary only. Entity→Domain mappers read values we wrote ourselves; a bad one there means a migration bug — different risk class, different change.

Verification

I can't compile here, so I checked the new strictness against existing data mechanically before committing: the stub's seed DTOs and every DTO construction across all test sources parse cleanly under the new rules (validated against the real enum constants), and no DTO reaches a mapper with a blank id. 8 new MappersTest cases pin each behaviour.

One consequence recorded, not hidden

map { it.toDomain() } over a list fails the whole read if one row is malformed — a single corrupt document empties a screen. That was already true of the previously-unguarded valueOf fields; uniform validation makes it uniformly true. Choosing between failing the read, dropping bad rows with a surfaced count, or returning partial results is a per-repository judgement across 13 call sites, so it's a ROADMAP Track C item rather than something to decide inside a mapper.

Docs

This also reconciles six documents that still described #24's problem as open — AUDIT_REPORT, README, ROADMAP, ARCHITECTURE, IMPLEMENTATION_SUMMARY and CLAUDE.md all still stated that every Firestore read fails. TECH_LEAD_REVIEW §9.4 is marked resolved with the original analysis kept as the record.

Verification

No Android SDK in this environment, so CI (assembleDebug, allUnitTests, assembleDebugAndroidTest) is the compile signal — please treat a red run as blocking. Base develop @ c692bc3.

🤖 Generated with Claude Code

https://claude.ai/code/session_01G6LvbheYzc1mN9toTuuPR5

---

Generated by Claude Code

[Copilot]

🟢 Ready to approve

The functional changes are well-scoped and backed by unit tests, with only minor non-blocking nits identified in review comments.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR tightens Firestore DTO → Domain mapping at the :data mapper boundary by adding explicit validation for enum fields and primary identity fields, so malformed/partial backend documents fail loudly and diagnosably instead of silently defaulting into incorrect domain objects. It also updates multiple docs to reflect that Firestore reads are now functional (post-PR #24) and that strictness is restored via mapper validation.

Changes:

• Add shared mapper helpers to validate required ids and parse enums with clear MalformedDtoException failures.
• Add JVM unit tests that pin the new validation behavior for representative DTOs.
• Update roadmap/audit/architecture/docs to mark the “Firestore reads fail” blocker as resolved and document the new validation stance.

File summaries

File	Description
TECH_LEAD_REVIEW.md	Marks §9.4 “Firestore reads fail” as resolved and documents the follow-up mapper validation.
ROADMAP.md	Updates Track C status (no-arg constructor fixed; mapper validation done; notes remaining degradation decision).
README.md	Updates Known Issues / warning text to reflect reads work and validation now exists.
IMPLEMENTATION_SUMMARY.md	Updates the implementation issues table to mark the read blocker as resolved and describe mapper validation.
data/src/test/java/com/example/diamonds/data/mapper/MappersTest.kt	Adds DTO-validation-focused unit tests for blank ids and unparseable enums.
data/src/main/java/com/example/diamonds/data/mapper/Mappers.kt	Introduces enumField/enumFieldOrNull/requiredId and applies them across DTO→Domain mappers.
core/src/main/java/com/example/diamonds/domain/model/Result.kt	Adds MalformedDtoException alongside existing domain-layer exceptions.
CLAUDE.md	Updates contributor guidance to reflect that reads work and mappers validate enums/ids.
AUDIT_REPORT.md	Updates audit summary/table to mark Firestore DTO deserialization as working and document mapper validation.
ARCHITECTURE.md	Updates backend swap notes and Track C status to reflect resolved DTO read blocker + validation behavior.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 2
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟢 Ready to approve

The mapper-boundary validation is consistently applied, covered by focused unit tests, and the documentation updates align with the implemented behavior.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.