Skip to content

fix(tests): model deactivation filtering - #1107

Merged
steebchen merged 1 commit into
mainfrom
terragon/fix-models-deactivation-filter-p9mdop
Nov 3, 2025
Merged

steebchen merged 1 commit into
mainfrom
terragon/fix-models-deactivation-filter-p9mdop

Conversation

@steebchen

@steebchen steebchen commented Nov 3, 2025

Copy link
Copy Markdown
Member

Summary

  • Aligns tests with corrected model deactivation semantics:
    • A model is excluded only if all providers are deactivated.
    • deactivated_at represents the earliest deactivation date among the model's providers.
    • Tests no longer assume future dates and verify a valid date if present.

Changes

Tests

  • Updated apps/gateway/src/models/models.spec.ts:
    • Remove strict current date comparison.
    • Add assertion that the response includes at least one model.
    • Validate deactivated_at is a valid ISO date string when present.

Behavior validated

  • Tests now reflect partial deactivations (some providers may be active) and still include models accordingly.

Rationale

  • Ensures tests align with the actual deactivation logic and avoid failures when some providers are active or dates are in the past.

Test plan

  • Run gateway unit tests and verify they pass
  • Verify models array contains items in the response
  • Validate deactivated_at, if present, is a valid ISO date

Documentation

  • Update relevant docs if necessary (no production code changes in this PR)

🌿 Generated by Terry


ℹ️ Tag @terragon-labs to ask questions and address PR feedback

📎 Task: https://www.terragonlabs.com/task/8574c8b6-1eb6-40d2-b5df-7c379e6d97c4

Summary by CodeRabbit

  • Tests
    • Revised deactivation date validation in test suite to verify date format validity instead of strict temporal comparisons.
    • Enhanced test coverage for model filtering with improved assertions.

Update tests to reflect that models with some deactivated providers but at least one active provider remain included in API responses. Verify response contains models and that deactivated_at dates are valid ISO strings if present, removing incorrect assumptions that all deactivated models are excluded.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@github-actions github-actions Bot changed the title Fix unit tests for model deactivation filtering semantics fix(tests): model deactivation filtering Nov 3, 2025
@coderabbitai

coderabbitai Bot commented Nov 3, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Test modifications in the models specification file that relax assertions on the deactivated_at field. The previous logic enforced strict date ordering relative to the current date; the updated logic validates that deactivated_at is a well-formed ISO date when present, without requiring temporal ordering constraints.

Changes

Cohort / File(s) Summary
Model deactivation test assertions
apps/gateway/src/models/models.spec.ts
Replaced current-date comparison validation for deactivated_at with ISO date validity checks; added clarification comment on deactivation semantics; added assertion confirming at least one model is returned in deactivated-filter test.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Test assertion changes are homogeneous and localized to a single file
  • Logic modification is straightforward (replacing one validation pattern with another)
  • Verify that the relaxed validation aligns with expected deactivated_at semantics across provider models

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'fix(tests): model deactivation filtering' clearly and concisely describes the main change in the changeset. It accurately reflects that the primary modification involves fixing unit tests related to model deactivation filtering logic. The title is specific enough to convey the focus on test corrections without being overly verbose, and it aligns well with the actual changes made to the models.spec.ts file.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch terragon/fix-models-deactivation-filter-p9mdop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/gateway/src/models/models.spec.ts (1)

66-67: Remove redundant instanceof Date check.

The check on line 66 is redundant: new Date() always returns a Date instance in JavaScript/TypeScript, even when the input string is invalid. Line 67 correctly validates that the date is actually parseable by checking isNaN(deactivatedAt.getTime()).

Apply this diff to remove the redundant check:

 			if (model.deactivated_at) {
 				const deactivatedAt = new Date(model.deactivated_at);
-				expect(deactivatedAt instanceof Date).toBe(true);
 				expect(isNaN(deactivatedAt.getTime())).toBe(false);
 			}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47309a7 and c94fe92.

📒 Files selected for processing (1)
  • apps/gateway/src/models/models.spec.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Always use top-level import; never use require() or dynamic import()

Files:

  • apps/gateway/src/models/models.spec.ts
**/*.spec.ts

📄 CodeRabbit inference engine (AGENTS.md)

Unit tests should be written in *.spec.ts files

Unit test files should be named with the .spec.ts suffix

Files:

  • apps/gateway/src/models/models.spec.ts
apps/{gateway,api}/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

apps/{gateway,api}/**/*.ts: Use Hono for HTTP routing in Gateway and API services
Use Zod schemas for request/response validation in server routes

Files:

  • apps/gateway/src/models/models.spec.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Never use any or as any in this TypeScript project unless absolutely necessary
Always use top-level import; do not use require or dynamic import()

Files:

  • apps/gateway/src/models/models.spec.ts
{apps/{api,gateway}/**/*.ts,packages/db/**/*.ts}

📄 CodeRabbit inference engine (CLAUDE.md)

For read operations, use db().query.<table>.findMany() or db().query.<table>.findFirst()

Files:

  • apps/gateway/src/models/models.spec.ts
🧠 Learnings (2)
📚 Learning: 2025-10-20T21:33:03.287Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-20T21:33:03.287Z
Learning: Applies to apps/gateway/src/api-individual.e2e.ts : Place isolated E2E test cases that need separation in `apps/gateway/src/api-individual.e2e.ts`

Applied to files:

  • apps/gateway/src/models/models.spec.ts
📚 Learning: 2025-09-29T21:46:22.366Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T21:46:22.366Z
Learning: Applies to apps/gateway/src/api-individual.e2e.ts : Place isolated/individual E2E test cases that need isolation in apps/gateway/src/api-individual.e2e.ts

Applied to files:

  • apps/gateway/src/models/models.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: test / run
  • GitHub Check: build / run
  • GitHub Check: generate / run
  • GitHub Check: lint / run
  • GitHub Check: e2e-shards (4)
  • GitHub Check: e2e-shards (5)
  • GitHub Check: e2e-shards (2)
  • GitHub Check: e2e-shards (1)
  • GitHub Check: e2e-shards (3)
  • GitHub Check: autofix
🔇 Additional comments (1)
apps/gateway/src/models/models.spec.ts (1)

52-60: LGTM! Clear documentation and sensible assertion.

The explanatory comment clarifies the deactivation semantics well, and the new assertion ensures the endpoint returns data as expected under the revised filtering logic.

@steebchen
steebchen merged commit 4e3a57b into main Nov 3, 2025
13 of 14 checks passed
@steebchen
steebchen deleted the terragon/fix-models-deactivation-filter-p9mdop branch November 3, 2025 13:04
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