Skip to content

fix(server): include daily_allocations in TaskFieldsBase server response model - #1172

Open
amir-rezaei wants to merge 1 commit into
Kohei-Wada:mainfrom
amir-rezaei:fix/server-task-operation-response-daily-allocations
Open

fix(server): include daily_allocations in TaskFieldsBase server response model#1172
amir-rezaei wants to merge 1 commit into
Kohei-Wada:mainfrom
amir-rezaei:fix/server-task-operation-response-daily-allocations

Conversation

@amir-rezaei

Copy link
Copy Markdown
Contributor

Description

This PR resolves issue #1135 by adding daily_allocations: dict[str, float] to TaskFieldsBase in packages/taskdog-server/src/taskdog_server/api/models/responses.py.

Details

  • Previously, TaskFieldsBase omitted daily_allocations, causing FastAPI response serialization of write operations (TaskOperationResponse, UpdateTaskResponse) to drop daily_allocations.
  • As a result, the client always validated and deserialized daily_allocations to an empty dictionary {} after create, update, or lifecycle calls.
  • Adding daily_allocations ensures full feature parity and complete data round-tripping between the server response models and client DTOs.

@Kohei-Wada

Copy link
Copy Markdown
Owner

Confirmed the underlying bug — TaskOperationOutput declares daily_allocations while TaskFieldsBase omits it, so the client validates it to {} on every write response. The fix is right.

Before merging, please add tests. This PR is a one-line field addition with no coverage, and #1135 explicitly asks for the drift guard rather than just the field:

  1. A drift test so the two hand-maintained shapes cannot silently diverge again:
    assert set(TaskOperationResponse.model_fields) == set(TaskOperationOutput.model_fields)
  2. A round-trip test: a create/update/lifecycle response carrying a non-empty daily_allocations should still be non-empty after the client deserializes it.

Verified locally against current main: lint, typecheck and the full suite pass (ruff is now 0.16, so please rebase).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants