Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make wpcom.req.* default response type unknown and add more zod schemas #899

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

fredrikekelund
Copy link
Contributor

Related issues

Proposed Changes

Our type definitions in src/custom-package-definitions.d.ts are very lax for the Request class in the WPCOM module – it sets the default response data type to any.

Now that we have zod in the codebase, defaulting to unknown is much more fitting since this forces us to validate the response data.

This PR makes that change, and fixes a few bugs I discovered along the way:

  • Our previous type definition for /studio-app/ai-assistant/chat API responses was incorrect. The topmost id prop is a number, not a string.
  • This means the Message[ 'chatApiId' ] type was also invalid (previously string, now number). Same thing for the chatApiIdDict state.
  • The chat Redux reducer previously stored prompt usage data in a promptUsageDict object, dividing it by site. This doesn't make sense since we limit prompt usage per WordPress.com user, not per site. I changed the state structure accordingly.

Testing Instructions

  1. Smoke test the Assistant
    1.1 Ensure previous conversations load as expected
    1.2 Ensure sending new messages works as expected
    1.3 Ensure the prompt usage meter in the settings modal updates as you send messages to the Assistant

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@fredrikekelund fredrikekelund requested a review from a team February 6, 2025 10:27
@fredrikekelund fredrikekelund self-assigned this Feb 6, 2025
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