fix: update helpdesk doubt parsing for new query_type schema

[pugal66]

What changed?

• Updated DoubtDto to read the query_type_display string from the API response instead of query_type.

Why?

• The backend API was updated to return an integer for query_type and a string for query_type_display. This ensures our parsing logic uses the new string field so that 'AI' and 'Mentor' doubts are identified correctly without breaking.

[claude]

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

---

Reviewed PR #232 — fix: update helpdesk doubt parsing for new query_type schema

• Read AGENTS.md, ai_context.md, architecture.md
• Fetch PR diff and read changed files
• Check OpenSpec compliance
• Check data layer / DTO parsing correctness
• Post structured review

PR Review Summary

✅ What's good

• The core fix is correct and minimal: DoubtDto.fromJson now reads query_type_display instead of the now-integer query_type (packages/core/lib/data/models/doubt_dto.dart:108), which matches the stated backend schema change and keeps the AI/Mentor string matching (== 'ai' / == 'mentor') working.
• Nice to see full OpenSpec docs (proposal.md, design.md, tasks.md, specs/helpdesk-doubts/spec.md) included even though this qualifies as a bug fix and is technically exempt from the OpenSpec requirement — good practice, and the design.md risk section is thoughtful (correctly flags the localized-display-string risk).
• Change is fully contained inside core's data layer (DTO parsing only) — no SDK boundary, widget, design-token, motion, navigation, or localization concerns triggered by this diff.
• HttpDataSource.createDoubt was correctly verified to already send an integer query_type in the POST body, so no unnecessary change was made there.

🔴 CRITICAL — Must fix before merge

None found.

🟡 WARNING — Should fix

1. Scope gap: doubt list filtering likely still broken. DoubtRepository.syncDoubts → HttpDataSource.getDoubts (packages/core/lib/data/sources/http_data_source.dart:698-708, called from doubt_repository.dart:52 with queryType: queryType?.name) still sends the string "ai"/"mentor" as the query_type query parameter when filtering the doubts list. Since the backend now treats query_type as an integer id (per this PR's own design.md), the filter request is plausibly broken the same way the DTO parsing was — it just isn't covered by this PR's scope. Worth a follow-up to verify/fix (mirroring the int mapping already used in createDoubt, doubt_repository.dart:78).
2. No regression test added. tasks.md marks 3.1 ("Run tests for DoubtDto if any exist") as done, but no DoubtDto unit tests exist in the repo (packages/core/test/). For a fix that's specifically about JSON field parsing, a small unit test asserting query_type_display: "AI" → DoubtQueryType.ai (and "Mentor" → .mentor) would guard against the next backend schema drift cheaply.

🔵 SUGGESTION — Nice to have

1. Consider updating the canonical openspec/specs/doubts-core/spec.md (line 36: "The parser SHALL extract the query_type attribute...") once this change is archived, since it currently documents the old field name — not required now since the delta spec correctly captures this, just a heads-up for the archive step.

Verdict

CHANGES REQUESTED — the DTO fix itself is correct and safe to ship, but the list-filter query-type gap (#1 above) is closely related to this exact schema migration and should at least be verified before merge to avoid a follow-up bug report; the missing test (#2) is a lower-severity nice-to-have.

[pugal66]

@claude review

[claude]

Claude finished @pugal66's task in 1m 31s —— View job

---

Reviewed PR #232 — fix: update helpdesk doubt parsing for new query_type schema

• Read AGENTS.md, ai_context.md, architecture.md
• Review latest diff (git diff HEAD~2..HEAD)
• Check whether prior review feedback was addressed
• Post structured review

PR Review Summary

This is a re-review after the latest commit (ff015cd "Address review comments"), which resolves both points raised in the previous round.

✅ What's good

• Filter-path gap fixed. DoubtRepository.syncDoubts now maps DoubtQueryType → integer before calling DataSource.getDoubts (packages/core/lib/data/repositories/doubt_repository.dart:52-54), consistent with the existing createDoubt mapping (doubt_repository.dart:80: ai → 2, mentor → 1). The DataSource/HttpDataSource/MockDataSource signatures were correctly updated in lockstep (String? queryType → int? queryType) across all three implementation points, keeping the abstract interface and both implementations in sync per the data-layer architecture rules.
• Regression test added. packages/core/test/data/models/doubt_dto_test.dart now covers both query_type_display: "AI" → DoubtQueryType.ai and "Mentor" → DoubtQueryType.mentor, closing the gap flagged earlier.
• HttpDataSource.getDoubts correctly only adds query_type to the query params when non-null (http_data_source.dart:707-709), and the int now serializes directly instead of the previous string.
• Change stays fully within core's data layer — no SDK boundary, widget, design-token, or navigation concerns triggered.

🔴 CRITICAL — Must fix before merge

None found.

🟡 WARNING — Should fix

None found — both prior warnings (filter path, missing test) have been addressed.

🔵 SUGGESTION — Nice to have

1. The queryType == DoubtQueryType.ai ? 2 : 1 mapping is now duplicated in two places (doubt_repository.dart:54 and :80). Consider extracting a small private helper (e.g. int? _queryTypeToId(DoubtQueryType? t)) to keep the id mapping in one place for the next schema change — not blocking.
2. openspec/changes/update-helpdesk-api-query-type/tasks.md section 2 only mentions verifying createDoubt; consider adding a line noting the getDoubts/syncDoubts filter-path fix for a more complete change record before archiving.
3. openspec/specs/doubts-core/spec.md:36 still documents the parser extracting the old query_type attribute — worth a heads-up for whoever archives this change to update the canonical spec.

Verdict

APPROVED — the DTO fix, the query-type filter fix, and the added regression test all look correct and consistent with existing conventions. Remaining items are minor polish only.
(branch)