Skip to content

feat(types): add core type system and Zod schemas#6

Merged
guycanella merged 2 commits into
mainfrom
feat/st-007-types-zod-schemas
Jul 6, 2026
Merged

feat(types): add core type system and Zod schemas#6
guycanella merged 2 commits into
mainfrom
feat/st-007-types-zod-schemas

Conversation

@guycanella

@guycanella guycanella commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds schemas/ with Zod-first type definitions across 8 domains: providers (LLMProvider, ModelInfo, ProviderConfig), chat (ChatMessage, StreamChunk, APIError), storage (UserSettings, APIKeyStore, WidgetPosition, RateLimitState), macros (PromptMacro, MacroLibrary), context detection (PageContext, ContextOverride), PII detection (PIITier, PIIPattern, PIIMatch), and telemetry (TelemetryEvent, TelemetryPayload).
  • RateLimitState was added beyond the original ST-007 scope by explicit request — it's documented in the technical-decisions doc alongside providers/APIError and will be needed by ST-031.
  • Every exported type is derived via z.infer<typeof XSchema> — Zod schemas are the single source of truth for both runtime validation and TypeScript types, ready for storage validation (ST-008), message-passing between content script/background/popup, and API response validation (ST-010/011).
  • Restricts TelemetryEvent.metadata to primitive values (string | number | boolean) instead of z.unknown(), as defense-in-depth against structured PII or large payloads riding along in a telemetry event.
  • Adds .env/.env.* to .gitignore (with a .env.example allowlist) ahead of upcoming API key handling work.

Notes for reviewers

  • This is foundational groundwork only — no entrypoints/ file imports any of these schemas yet. Actual usage lands starting with ST-008 (storage layer) and ST-010 (provider factory).
  • Hit a TypeScript inference quirk in APIKeyStore's .refine(): Object.values() on an object with optional properties can silently drop undefined from the inferred element type, which made @typescript-eslint/no-unnecessary-condition flag a legitimate null-check as dead code. Fixed by checking each field directly instead of iterating.
  • No test runner exists yet in this repo, so schema behavior (enum validation, the "at least one API key" refine, required-field rejection) was verified manually via a throwaway Node smoke script rather than an automated test.
  • Security review (Lucas): PASS across both review passes — no Critical/High findings.

Test plan

  • pnpm run compile — no type errors
  • pnpm run lint — zero errors
  • pnpm run format:check — all files match Prettier style
  • pnpm run build — MV3 output builds correctly, bundle size unchanged (nothing consumes these schemas yet)
  • Manual: ad-hoc Node script confirmed LLMProviderSchema, APIKeyStoreSchema, WidgetPositionSchema, and UserSettingsSchema accept valid input and reject invalid input as expected

Refs: ST-007

Summary by CodeRabbit

  • New Features
    • Added consistent, structured validation for chat messages (including streaming and error details), prompt macros, page context settings, provider configuration, persisted storage settings, PII tiering/matching, and telemetry events.
    • Expanded shared schema exports so these validated data shapes can be reused across app features.
  • Bug Fixes
    • Updated environment file handling to ignore .env and .env.* by default, while keeping .env.example tracked.

Add schemas/ with Zod-first type definitions for the seven ST-007
domains plus rate limiting: providers (LLMProvider, ModelInfo,
ProviderConfig), messages (ChatMessage, StreamChunk, APIError),
storage (UserSettings, APIKeyStore, WidgetPosition, RateLimitState),
macros (PromptMacro, MacroLibrary), context detection (PageContext,
ContextOverride), PII detection (PIITier, PIIPattern, PIIMatch), and
telemetry (TelemetryEvent, TelemetryPayload). Every type is derived
via z.infer from its Zod schema, keeping validation and typing as a
single source of truth for later storage, message-passing, and API
response validation.

Restrict TelemetryEvent.metadata values to primitives (string, number,
boolean) instead of z.unknown(), as defense-in-depth against
structured PII or large payloads riding along in a telemetry event.
Add .env/.env.* to .gitignore (with a .env.example allowlist) ahead of
API key handling work.

This is foundational groundwork only - no entrypoint consumes these
schemas yet; usage lands starting with ST-008 (storage) and ST-010
(provider factory).

Refs: ST-007
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds Zod-based schemas and inferred types for chat, context, macros, PII, providers, storage, and telemetry, re-exports them from a schema barrel, and updates .gitignore to exclude environment files while keeping .env.example tracked.

Changes

Schema Definitions

Layer / File(s) Summary
Provider config schemas
schemas/provider.ts
Defines LLM provider, model info, and provider configuration schemas and inferred types.
Chat message and streaming schemas
schemas/chat.ts
Defines chat role, chat message, stream chunk, API error kind, and API error schemas and inferred types.
User settings and storage schemas
schemas/storage.ts
Defines theme, PII region and scanning mode, keyboard shortcuts, user settings, API key store, widget position, and rate-limit state schemas and inferred types.
PII, context, and macro schemas
schemas/pii.ts, schemas/context.ts, schemas/macro.ts
Defines PII tier, pattern, and match schemas, page context and context override schemas, and prompt macro and macro library schemas.
Telemetry event schemas
schemas/telemetry.ts
Defines telemetry event type, metadata value, telemetry event, and telemetry payload schemas and inferred types.
Schema barrel export and env gitignore
schemas/index.ts, .gitignore
Re-exports schema modules and updates .gitignore for .env files while preserving .env.example.

Estimated code review effort: 2 (Simple) | ~12 minutes

Poem

A rabbit found a Zod-lit grove,
With schemas sprouting row by row;
Chat and telemetry now sing,
PII, macros, storage in a ring,
And .env nibbles stay outside the woe. 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding foundational types and Zod schemas.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/st-007-types-zod-schemas

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

schemas/pii.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

schemas/storage.ts

ESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.

schemas/telemetry.ts

ESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@schemas/pii.ts`:
- Around line 14-22: The PIIMatchSchema currently accepts any numeric startIndex
and endIndex, so malformed matches can slip through and break masking logic. Add
validation to PIIMatchSchema to enforce non-negative indices and require
endIndex to be greater than startIndex, using a schema-level refinement on the
PIIMatchSchema object. Keep the constraint close to the existing
PIIMatchSchema/PIIMatch type definitions so invalid PIIMatch values are rejected
before substring slicing occurs.

In `@schemas/storage.ts`:
- Around line 29-44: APIKeyStoreSchema currently hardcodes provider keys, which
duplicates the provider list and can drift from the enum. Update the schema in
schemas/storage.ts to derive its object shape from LLMProviderSchema using
z.partialRecord(LLMProviderSchema, z.string().min(1)), and keep the existing “at
least one API key is required” refine. Make sure APIKeyStore continues to infer
from the updated schema so all provider keys stay in sync automatically.

In `@schemas/telemetry.ts`:
- Line 22: The anonymousUuid schema is too permissive because it currently uses
z.string(), so update the telemetry schema to validate it as a real UUID using
Zod’s uuid validator. Make the change in the telemetry schema definition where
anonymousUuid is declared so the field name and validation rule match, and keep
the rest of the schema unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f328c10f-393c-4baa-a9ac-5c4d0fe838b7

📥 Commits

Reviewing files that changed from the base of the PR and between 0bdaa2f and ff7476a.

📒 Files selected for processing (9)
  • .gitignore
  • schemas/chat.ts
  • schemas/context.ts
  • schemas/index.ts
  • schemas/macro.ts
  • schemas/pii.ts
  • schemas/provider.ts
  • schemas/storage.ts
  • schemas/telemetry.ts

Comment thread schemas/pii.ts Outdated
Comment thread schemas/storage.ts
Comment thread schemas/telemetry.ts Outdated
Add ordering/bounds validation to PIIMatchSchema (non-negative integer
indices, endIndex strictly greater than startIndex) to prevent a
malformed match from later causing invalid substring slicing during
PII masking.

Derive APIKeyStoreSchema from LLMProviderSchema via z.partialRecord
instead of hand-duplicating the four provider names, which as a side
effect now rejects unknown/misspelled provider keys instead of
silently stripping them.

Validate TelemetryEvent.anonymousUuid as an actual UUID (z.uuid())
instead of an unconstrained string, matching its documented purpose.

Refs: ST-007

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
schemas/storage.ts (1)

42-48: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Tighten numeric constraints on RateLimitStateSchema.

requestCount, windowStart, and nextAvailableAt are plain z.number(), allowing negative or fractional values that don't make sense for a request counter or epoch timestamp.

♻️ Suggested tightening
 export const RateLimitStateSchema = z.object({
   provider: LLMProviderSchema,
-  requestCount: z.number(),
-  windowStart: z.number(),
-  nextAvailableAt: z.number(),
+  requestCount: z.number().int().nonnegative(),
+  windowStart: z.number().int().nonnegative(),
+  nextAvailableAt: z.number().int().nonnegative(),
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@schemas/storage.ts` around lines 42 - 48, Tighten the numeric validation in
RateLimitStateSchema so requestCount, windowStart, and nextAvailableAt only
accept sensible non-negative whole-number values instead of any number. Update
the RateLimitStateSchema definition in schemas/storage.ts to use stricter
constraints for these fields, and keep the existing RateLimitState type
inference unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@schemas/storage.ts`:
- Around line 42-48: Tighten the numeric validation in RateLimitStateSchema so
requestCount, windowStart, and nextAvailableAt only accept sensible non-negative
whole-number values instead of any number. Update the RateLimitStateSchema
definition in schemas/storage.ts to use stricter constraints for these fields,
and keep the existing RateLimitState type inference unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: dd203446-6d49-4490-832a-8310dc0ac556

📥 Commits

Reviewing files that changed from the base of the PR and between ff7476a and fa1f0f2.

📒 Files selected for processing (3)
  • schemas/pii.ts
  • schemas/storage.ts
  • schemas/telemetry.ts

@guycanella
guycanella merged commit 77dba37 into main Jul 6, 2026
1 check passed
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