Skip to content

fix migration tests #2371

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.

Sign up for GitHub

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

Merged
akshaydeo merged 1 commit into from
Mar 29, 2026
Merged

fix migration tests #2371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 42 additions & 8 deletions .github/workflows/scripts/run-migration-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,10 +481,10 @@ VALUES
ON CONFLICT DO NOTHING;

-- config_providers (with all JSON config fields and governance fields including budget_id, rate_limit_id)
INSERT INTO config_providers (name, send_back_raw_request, send_back_raw_response, network_config_json, concurrency_buffer_json, proxy_config_json, custom_provider_config_json, budget_id, rate_limit_id, config_hash, created_at, updated_at)
VALUES
('openai', false, false, '{"timeout": 30}', '{"buffer_size": 100}', NULL, NULL, 'budget-migration-test-1', 'ratelimit-migration-test-1', 'provider-hash-openai', $now, $now),
('anthropic', true, true, '{"timeout": 60}', '{"buffer_size": 200}', '{"url": "http://proxy.test"}', NULL, NULL, NULL, 'provider-hash-anthropic', $now, $now)
INSERT INTO config_providers (name, send_back_raw_request, send_back_raw_response, network_config_json, concurrency_buffer_json, proxy_config_json, custom_provider_config_json, open_ai_config_json, budget_id, rate_limit_id, config_hash, created_at, updated_at)
VALUES
('openai', false, false, '{"timeout": 30}', '{"buffer_size": 100}', NULL, NULL, '{"organization": "org-test"}', 'budget-migration-test-1', 'ratelimit-migration-test-1', 'provider-hash-openai', $now, $now),
('anthropic', true, true, '{"timeout": 60}', '{"buffer_size": 200}', '{"url": "http://proxy.test"}', NULL, NULL, NULL, NULL, 'provider-hash-anthropic', $now, $now)
ON CONFLICT DO NOTHING;
Comment on lines +484 to 488
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 29, 2026

Choose a reason for hiding this comment

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

P2 New columns in static INSERT break pre-v1.4.17 schema tests

open_ai_config_json (and blacklisted_models_json in config_keys) are now listed in the static INSERT column list. When the migration test runs against any schema older than v1.4.17 — where these columns don't yet exist — PostgreSQL will throw ERROR: column "open_ai_config_json" of relation "config_providers" does not exist, causing the entire INSERT to fail. All subsequent config_keys INSERTs that SELECT … FROM config_providers WHERE name = 'openai' will then silently insert zero rows as well.

The existing pattern for exactly this scenario is the azure_scopes treatment (see the NOTE comment on the config_keys INSERT): keep the new column out of the static INSERT and supply its value only inside append_dynamic_columns_postgres/sqlite guarded by column_exists_*. That way the static INSERT succeeds on all older schemas, and the extra data is only emitted when the column actually exists.

The same fix applies to the two config_keys INSERTs that now include blacklisted_models_json.


-- framework_configs
Expand Down Expand Up @@ -563,13 +563,13 @@ ON CONFLICT DO NOTHING;

-- config_keys (references config_providers) - with ALL columns including Azure/Vertex/Bedrock/Replicate fields
-- NOTE: azure_scopes column is added dynamically via append_dynamic_inserts() for schema compatibility
INSERT INTO config_keys (name, provider_id, provider, key_id, value, models_json, weight, enabled, config_hash, azure_endpoint, azure_api_version, azure_deployments_json, azure_client_id, azure_client_secret, azure_tenant_id, vertex_project_id, vertex_project_number, vertex_region, vertex_auth_credentials, vertex_deployments_json, bedrock_access_key, bedrock_secret_key, bedrock_session_token, bedrock_region, bedrock_arn, bedrock_deployments_json, bedrock_batch_s3_config_json, use_for_batch_api, replicate_deployments_json, created_at, updated_at)
SELECT 'migration-test-key-openai', id, 'openai', 'key-migration-uuid-001', 'sk-migration-test-fake-key-value-openai', '["gpt-4", "gpt-3.5-turbo"]', 1.0, true, 'key-hash-001', NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, NULL, $now, $now
INSERT INTO config_keys (name, provider_id, provider, key_id, value, models_json, blacklisted_models_json, weight, enabled, config_hash, azure_endpoint, azure_api_version, azure_deployments_json, azure_client_id, azure_client_secret, azure_tenant_id, vertex_project_id, vertex_project_number, vertex_region, vertex_auth_credentials, vertex_deployments_json, bedrock_access_key, bedrock_secret_key, bedrock_session_token, bedrock_region, bedrock_arn, bedrock_deployments_json, bedrock_batch_s3_config_json, use_for_batch_api, replicate_deployments_json, created_at, updated_at)
SELECT 'migration-test-key-openai', id, 'openai', 'key-migration-uuid-001', 'sk-migration-test-fake-key-value-openai', '["gpt-4", "gpt-3.5-turbo"]', '["gpt-4-32k"]', 1.0, true, 'key-hash-001', NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, NULL, $now, $now
FROM config_providers WHERE name = 'openai'
ON CONFLICT DO NOTHING;

INSERT INTO config_keys (name, provider_id, provider, key_id, value, models_json, weight, enabled, config_hash, azure_endpoint, azure_api_version, azure_deployments_json, azure_client_id, azure_client_secret, azure_tenant_id, vertex_project_id, vertex_project_number, vertex_region, vertex_auth_credentials, vertex_deployments_json, bedrock_access_key, bedrock_secret_key, bedrock_session_token, bedrock_region, bedrock_arn, bedrock_deployments_json, bedrock_batch_s3_config_json, use_for_batch_api, replicate_deployments_json, created_at, updated_at)
SELECT 'migration-test-key-anthropic', id, 'anthropic', 'key-migration-uuid-002', 'sk-ant-migration-test-fake-key', '["claude-3-opus"]', 0.8, true, 'key-hash-002', NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, NULL, $now, $now
INSERT INTO config_keys (name, provider_id, provider, key_id, value, models_json, blacklisted_models_json, weight, enabled, config_hash, azure_endpoint, azure_api_version, azure_deployments_json, azure_client_id, azure_client_secret, azure_tenant_id, vertex_project_id, vertex_project_number, vertex_region, vertex_auth_credentials, vertex_deployments_json, bedrock_access_key, bedrock_secret_key, bedrock_session_token, bedrock_region, bedrock_arn, bedrock_deployments_json, bedrock_batch_s3_config_json, use_for_batch_api, replicate_deployments_json, created_at, updated_at)
SELECT 'migration-test-key-anthropic', id, 'anthropic', 'key-migration-uuid-002', 'sk-ant-migration-test-fake-key', '["claude-3-opus"]', '[]', 0.8, true, 'key-hash-002', NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, NULL, $now, $now
FROM config_providers WHERE name = 'anthropic'
ON CONFLICT DO NOTHING;

Expand Down Expand Up @@ -1189,6 +1189,22 @@ append_dynamic_columns_postgres() {
echo "UPDATE governance_model_pricing SET code_interpreter_cost_per_session = NULL WHERE id = 1;" >> "$output_file"
echo "UPDATE governance_model_pricing SET code_interpreter_cost_per_session = NULL WHERE id = 2;" >> "$output_file"
fi

# -------------------------------------------------------------------------
# v1.4.17 columns
# -------------------------------------------------------------------------

# config_keys.blacklisted_models_json (added in v1.4.17 - per-key model deny list)
if column_exists_postgres "config_keys" "blacklisted_models_json"; then
echo "UPDATE config_keys SET blacklisted_models_json = '[]' WHERE name = 'migration-test-key-openai';" >> "$output_file"
echo "UPDATE config_keys SET blacklisted_models_json = '[\"gpt-4-vision\"]' WHERE name = 'migration-test-key-anthropic';" >> "$output_file"
fi

# config_providers.open_ai_config_json (added in v1.4.17 - OpenAI-specific provider config)
if column_exists_postgres "config_providers" "open_ai_config_json"; then
echo "UPDATE config_providers SET open_ai_config_json = '{\"disable_store\":false}' WHERE name = 'openai';" >> "$output_file"
echo "UPDATE config_providers SET open_ai_config_json = '' WHERE name = 'anthropic';" >> "$output_file"
Comment on lines +1205 to +1206
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 29, 2026

Choose a reason for hiding this comment

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

P2 Empty string stored in JSON field for non-OpenAI provider

open_ai_config_json is a text column (not json/jsonb), so PostgreSQL accepts an empty string without a type error. The application-side deserialization logic (in tables/provider.go) treats "" and NULL identically, so this is functionally correct. However, the static INSERT already stores NULL for 'anthropic'; the dynamic UPDATE then overwrites it with ''. Using NULL in both places would be consistent.

The same applies to the SQLite dynamic section at line ~1672.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

fi
}

# Append dynamic column UPDATEs for columns that may not exist in older schemas (SQLite)
Expand Down Expand Up @@ -1638,6 +1654,24 @@ append_dynamic_columns_sqlite() {
echo "UPDATE logs SET cached_read_tokens = 128 WHERE id = 'log-migration-test-001';" >> "$output_file"
echo "UPDATE logs SET cached_read_tokens = 0 WHERE id = 'log-migration-test-002';" >> "$output_file"
echo "UPDATE logs SET cached_read_tokens = 0 WHERE id = 'log-migration-test-003';" >> "$output_file"

# -------------------------------------------------------------------------
# v1.4.17 columns
# -------------------------------------------------------------------------

if [ -f "$config_db" ]; then
# config_keys.blacklisted_models_json (added in v1.4.17 - per-key model deny list)
if column_exists_sqlite "$config_db" "config_keys" "blacklisted_models_json"; then
echo "UPDATE config_keys SET blacklisted_models_json = '[]' WHERE name = 'migration-test-key-openai';" >> "$output_file"
echo "UPDATE config_keys SET blacklisted_models_json = '[\"gpt-4-vision\"]' WHERE name = 'migration-test-key-anthropic';" >> "$output_file"
fi

# config_providers.open_ai_config_json (added in v1.4.17 - OpenAI-specific provider config)
if column_exists_sqlite "$config_db" "config_providers" "open_ai_config_json"; then
echo "UPDATE config_providers SET open_ai_config_json = '{\"disable_store\":false}' WHERE name = 'openai';" >> "$output_file"
echo "UPDATE config_providers SET open_ai_config_json = '' WHERE name = 'anthropic';" >> "$output_file"
fi
fi
}

# ============================================================================
Expand Down
4 changes: 2 additions & 2 deletions core/internal/llmtests/validation_presets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,8 +476,8 @@ func ModifyExpectationsForProvider(expectations ResponseExpectations, provider s

case schemas.Perplexity:
expectations.ShouldHaveUsageStats = true
expectations.ShouldHaveTimestamps = true
expectations.ShouldHaveModel = true
expectations.ShouldHaveTimestamps = false // Perplexity does not return created timestamps
expectations.ShouldHaveModel = false // Perplexity does not return model field
expectations.ShouldHaveLatency = true

case schemas.Cerebras:
Expand Down
5 changes: 5 additions & 0 deletions transports/config.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,11 @@
"type": "string",
"format": "date-time",
"description": "Last time budget was reset"
},
"calendar_aligned": {
"type": "boolean",
"description": "Snap resets to calendar boundaries (day/week/month/year start)",
"default": false
}
},
"required": [
Expand Down
Loading