Skip to content

fix: harden pyright strict-mode follow-up regressions#336

Draft
CCimen wants to merge 1 commit intodevelopfrom
feature/pr325-strict-safety-fixes
Draft

fix: harden pyright strict-mode follow-up regressions#336
CCimen wants to merge 1 commit intodevelopfrom
feature/pr325-strict-safety-fixes

Conversation

@CCimen
Copy link
Copy Markdown
Contributor

@CCimen CCimen commented Apr 9, 2026

Summary

Follow-up to #325.

This PR fixes a small set of runtime and contract regressions that surfaced after the backend-wide Pyright strict migration, while keeping the strict baseline intact.

What changed

  • keep assistant session pagination cursors nullable instead of coercing missing cursors to datetime.min
  • preserve nullable tool_call_id values in assistant SSE and persisted tool metadata instead of serializing empty strings
  • require a real approval_id for approval SSE events instead of silently emitting ""
  • register audit worker task names in the Task enum and replace cast(Task, "...") with real enum members
  • centralize optional datetime fallback handling in a UTC-aware helper and reuse it in the strict-mode sort paths that previously mixed None, naive datetimes, and aware datetimes

Why

PR #325 made the backend globally strict, but a few pyright-driven fallbacks changed runtime behavior:

  • assistant pagination changed previous_cursor from null to 0001-01-01T00:00:00
  • assistant tool approval events changed nullable IDs to empty strings on the wire
  • audit worker task names were hidden behind unsafe enum casts
  • one created_at sort path could still crash at runtime, and the straightforward datetime.min fallback is not safe when real timestamps are timezone-aware

This PR restores the original API semantics where appropriate and hardens the datetime fallback so it is correct at runtime.

Testing

Targeted verification run in the devcontainer:

  • cd /workspace && set -a && . backend/.env && set +a && cd backend && python -m pytest -q tests/unittests/ai_models/test_ai_models_service.py tests/unittests/spaces/test_space.py tests/unittests/assistants/test_assistant_router.py tests/unittests/questions/test_questions_repo.py tests/unittests/worker/test_worker.py tests/unit/test_assistant_protocol.py tests/unit/test_audit_async.py
  • cd /workspace && set -a && . backend/.env && set +a && cd backend && python -m pytest -q tests/unittests/sessions/test_protocol.py tests/unit/test_conversations_tool_approval_contract.py tests/unit/test_tool_approval_manager.py tests/unit/test_completion_service_streaming.py tests/unit/test_export_job_manager.py tests/integration/audit/test_audit_worker.py tests/integration/test_tool_approval_endpoint.py
  • cd /workspace/backend && ./scripts/run_pyright_in_devcontainer.sh src/intric/ai_models/ai_models_service.py src/intric/assistants/api/assistant_router.py src/intric/assistants/api/assistant_protocol.py src/intric/assistants/assistant_service.py src/intric/jobs/job_models.py src/intric/api/audit/routes.py src/intric/audit/application/audit_service.py src/intric/main/datetime_utils.py src/intric/spaces/space.py src/intric/spaces/space_service.py src/intric/completion_models/application/completion_model_crud_service.py src/intric/transcription_models/domain/transcription_model_service.py src/intric/transcription_models/application/transcription_model_crud_service.py src/intric/group_chat/presentation/assemblers/group_chat_assembler.py

Notes

Restore the assistant and audit contracts that drifted during the pyright strict migration and harden Optional datetime sorting so it is actually safe at runtime.

What this changes:
- keep assistant session pagination cursors nullable instead of coercing missing cursors to datetime.min
- preserve nullable tool_call_id values in SSE and persisted assistant tool metadata instead of emitting empty strings
- require a real approval_id for approval SSE events instead of silently serializing an empty string
- register audit worker task names in the Task enum and stop hiding them behind cast(Task, "...")
- centralize Optional datetime fallback logic in a UTC-aware helper and use it in the strict-mode sort paths that previously mixed None, naive datetimes, and aware datetimes

Why:
- fixes the runtime sort crash risk in ai_models_service and the same latent aware-vs-naive issue in adjacent strict-mode sort sites
- restores pre-migration API and wire semantics for nullable pagination and tool approval fields
- makes the Task typing honest instead of masking enum gaps with casts
- adds focused regression coverage for the touched safety paths
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.

1 participant