Skip to content

fix(model): drop leftover prefill_groups unique constraints - #7100

Merged
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
seefs001:fix/gorm-pg-index
Aug 30, 2026
Merged

fix(model): drop leftover prefill_groups unique constraints#7100
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
seefs001:fix/gorm-pg-index

Conversation

@seefs001

@seefs001 seefs001 commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

⚠️ 提交说明 / PR Notice

English template: .github/PULL_REQUEST_TEMPLATE/en.md

Important

  • 描述可用 AI 辅助。提交前请审阅全文,并声明对其负责,避免未经核对的直接粘贴。
  • 请按本模板填写后再提交。

🔗 关联任务 / Related Issue

  • 新功能请填写下方 Issue 编号;若还没有对应 Issue,请先自行创建。功能讨论请放在 Issue 中进行。

  • 改动较大或方向性变更,请先在关联 Issue 中与维护者达成一致,再提交 PR。

  • Bug 修复请关联对应 Issue。设计取舍、理解偏差或预期不一致,更适合作为讨论或功能请求。

  • Closes #

🚀 变更类型 / Type of change

  • 🐛 Bug 修复 (Bug fix)
  • ✨ 新功能 (New feature)
  • ⚡ 性能优化 / 重构 (Refactor)
  • 📝 文档更新 (Documentation)

📝 变更描述 / Description

(简述做了什么、为什么生效。如果难以简述,建议先拆分范围,或在 Issue 中与维护者对齐。)

在gorm自动迁移前清理掉错误的index

📸 运行证明 / Proof of Work

(请写明如何验证:实际步骤与观察结果。UI 变更请附截图或录屏;Bug 修复请说明复现过程与修复后结果。)

✅ 提交前检查项 / Checklist

  • 人工确认: 无论描述是否由 AI 生成,我已审阅全部内容,并声明对其准确性与完整性负责。
  • 非重复提交: 我已搜索现有的 IssuesPRs,确认不是重复提交。
  • 新功能关联 Issue: 若此 PR 标记为 New feature,我已关联对应 Issue;若尚无 Issue,我已先自行创建。
  • 事前沟通: 若改动较大或涉及方向性变更,已在关联 Issue 中与维护者沟通并达成一致。
  • 功能范围: 本 PR 不是 Coding Plan、逆向渠道、第三方封装接口,也不是对 Codex 渠道类型的改动。
  • 范围聚焦: 本 PR 为一项聚焦改动,未包含无关代码。
  • 本地验证: 已在本地运行并通过测试或手动验证,维护者可以据此复核。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

Summary by CodeRabbit

  • Bug Fixes
    • Improved PostgreSQL database migrations by safely removing obsolete uniqueness rules from prefill group names.
    • Prevented migration failures caused by leftover legacy constraints or indexes.
    • Preserved the intended unique-name behavior during database upgrades.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4c7bf4a6-9378-4f18-96e4-d7eb862a300e

📥 Commits

Reviewing files that changed from the base of the PR and between 7415871 and c43d824.

📒 Files selected for processing (2)
  • model/main.go
  • model/prefill_group_migration.go

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

The migration flow adds PostgreSQL-specific cleanup for legacy unique constraints and indexes on prefill_groups.name. Both standard and fast migration paths run this cleanup before GORM migrations and propagate errors.

Changes

Prefill group unique index migration

Layer / File(s) Summary
Legacy unique cleanup
model/prefill_group_migration.go
The new helpers restrict cleanup to PostgreSQL databases with an existing PrefillGroup table. They remove single-column name unique constraints and legacy unique indexes while preserving uk_prefill_name.
Migration entrypoint integration
model/main.go
migrateDB() and migrateDBFast() invoke the cleanup before their migration operations and return cleanup errors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to c43d8

The PR adds localized cleanup of legacy PostgreSQL PrefillGroup unique constraints before database migration; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: calcium-ion

Poem

A rabbit checks the schema at dawn

Old name keys quietly move on
Constraints drop with careful grace
Fast and slow paths share the place
Then migrations hop along

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing leftover unique constraints from the prefill_groups table during migration.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@Calcium-Ion
Calcium-Ion merged commit 69a41ee into QuantumNous:main Aug 30, 2026
2 of 3 checks passed
chunfeng789 added a commit to chunfeng789/new-api that referenced this pull request Aug 30, 2026
* fix(web): restore admin unbinding for built-in providers (QuantumNous#6987)

* fix(web): align admin binding types

Refs QuantumNous#6985

* test(web): restore animation mock

* fix(billing): 修复时间规则恒真表达式导致倍率全天生效 (QuantumNous#6934)


Co-authored-by: seefs001 <i@seefs.me>

* fix(docker): add relaykit go.mod to dev build context (QuantumNous#7072)

* feat(task): replace built-in task adaptors with a sandboxed JS plugin system (QuantumNous#7076)

* fix(relay): 请求参数校验错误返回 HTTP 400 (QuantumNous#6774)

* fix(relay): return 400 for invalid request parameters

* fix(web): recheck setup status after page reload (QuantumNous#6968)

* feat(auth): encrypt password login transport

Closes QuantumNous#6743

* feat(chat): add AQBot preset (QuantumNous#7079)

* feat(auth): make password encryption opt-in QuantumNous#6743

* feat(task): resolve channel-mapped aliases and case variants for plugin models

Channel model_mapping keys exposed in a channel's model list now act as
first-class aliases for task-plugin models across the whole line:

- Derived alias view (model/task_model_alias.go): built from enabled
  channels' model_mapping, chain-following with cycle detection, declared
  names always win, cross-plugin conflicts dropped. Rebuilt on channel
  cache refresh, registry generation change, and a 60s TTL.
- Request path: PinTaskPluginEndpoint resolves declared-name case folds
  and mapping aliases before endpoint lookup (never rewriting the body
  until the endpoint is claimed), pins with MappedModel, and the decode
  contract accepts alias echoes without loosening model ownership for
  normal pins. Legacy /v1/tasks submit folds case variants the same way.
  Fixes aliases on POST /v1/responses silently falling through to the
  main relay against task channels.
- Mapping order: ModelMappedHelper now runs before the plugin submit
  hook builds and caches the upstream body, so channel model_mapping
  actually reaches the upstream request. Plugins receive the mapped
  name as ctx.upstreamModel in both decode and submit contexts.
- Billing: identity stays the origin name; when the alias has no tiered
  expression, the selected channel's mapping tail expression applies.
  Pricing page and billing-expr smoke tests resolve aliases to the
  owning plugin's usage schema.
- Case folding: ASCII-only fold with exact-match priority; same-plugin
  and cross-plugin fold collisions rejected at registration.
- Plugins: model-keyed rate tables, req_key derivation, and combo
  validation in doubao/kling/jimeng/hailuo/vidu/sunoapi now key on
  ctx.upstreamModel || ctx.model; render/echo paths keep ctx.model.

* fix(model): disable PostgreSQL prepared statements for pooler compatibility

GORM v1.25.2 closes cached prepared statements asynchronously on any SQL
error and immediately re-Parses the same deterministic name (pgx's
stmt_<sha256>) on the same client connection. Transaction-pooling proxies
(PgBouncer >=1.21 with max_prepared_statements, Neon, Supabase) respond
with FATAL "prepared statement name is already in use" (SQLSTATE 08P01)
and drop the connection. PreferSimpleProtocol only disables pgx's
implicit prepare and never covered GORM's explicit PrepareStmt cache.

- PostgreSQL now runs with PrepareStmt disabled entirely; named prepared
  statements are fundamentally session state and cannot be made safe
  under transaction pooling. Parse/plan cost is noise for this workload.
- Upgrade gorm to v1.25.12 so MySQL/SQLite statement caches (still
  enabled) no longer churn close/re-prepare on ordinary SQL errors;
  v1.25.9+ restricts eviction to driver.ErrBadConn. Deliberately not
  v1.26+, whose LRU eviction has an open use-after-close race (#7831).
- sanitizeDBError now attaches a remediation hint on 08P01/42P05 so
  affected deployments can self-diagnose from the log line.

* fix(ali): honor image response format (QuantumNous#5513) (QuantumNous#7048)

* feat(web): factory task plugins update only with the system

Marketplace install/upgrade on a factory-served plugin actually created
a permanent override shadowing every future built-in release. The card
now shows an informational "Updates with the system" badge instead of
the action, while keeping the built-in vs marketplace version line and
the upgradable state badge visible. Deliberate overrides are untouched:
upload and marketplace actions on overridden or third-party plugins
behave as before, and the plugins table now hints when an override
lags behind the shipped built-in version so operators know deleting it
restores the newer factory plugin.

* fix(subscription): 无有效订阅时前端如实显示「仅用订阅」偏好 (QuantumNous#6222) (QuantumNous#7086)

Co-authored-by: Claude <noreply@anthropic.com>

* fix(sqlite): enable WAL + working busy timeout + _txlock=immediate to stop concurrent write lockouts (QuantumNous#7030)

* fix(sqlite): enable WAL + working busy timeout + _txlock=immediate to stop concurrent write lockouts

* fix(model): return string from JSON column Valuers for pg simple protocol

With PrepareStmt disabled, PostgreSQL queries run over pgx's simple
protocol, which encodes every []byte parameter as a bytea hex literal
('\x...'). driver.Valuer implementations returning []byte from
json.Marshal therefore fail json-column writes with SQLSTATE 22P02
(reported on the channels UPDATE path via ChannelInfo).

Reproduced against a live PostgreSQL 16: []byte Valuer into a json
column fails under simple protocol, string succeeds; []byte into a
text column silently stores the hex literal (no such path exists in
the repo today — audited all Valuers, json.RawMessage fields, and raw
SQL call sites).

- ChannelInfo, Properties, TaskPrivateData, JSONValue Value() now
  return string; zero-value nil semantics unchanged. Task.Data
  (bare json.RawMessage) is unaffected — database/sql's default
  converter already passes it as expected.
- Their Scan() counterparts now accept both []byte and string via a
  shared jsonScanBytes helper: SQLite returns string for these columns
  once Value() emits string, and the old []byte-only assertions
  silently zeroed the field (caught by the model test suite).
- Add regression tests locking both contracts: json-column Valuers
  must return string (or nil for zero values), Scanners must accept
  []byte and string.

Verified end-to-end against PostgreSQL 16 with the real model types:
Channel create/update/read-back, Task json fields, PrefillGroup items.

* fix(relay): bound the wait for upstream response headers (fixes unbounded heap growth → OOM) (QuantumNous#6949)

* fix(relay): bound the wait for upstream response headers (fixes unbounded heap growth)

The relay transport sets a dial timeout, a TLS handshake timeout and an expect-continue
timeout, but nothing bounds how long it waits for the upstream *response headers* after
the request has been written. An upstream that accepts the connection and then never
answers -- without sending FIN/RST, which is what happens when a NAT/firewall silently
drops the flow or the provider hangs -- parks the goroutine in
net/http.(*persistConn).roundTrip forever.

That goroutine keeps the whole request alive, which in practice means three copies of the
request body stay reachable for the lifetime of the process: the raw bytes from
io.ReadAll in CreateBodyStorageFromReader, the decoded messages held as json.RawMessage,
and the re-marshalled upstream body from common.Marshal. BodyStorageCleanup cannot help
here: it runs after c.Next() returns, and for these requests c.Next() never returns.

Measured on v1.0.0-rc.23 in production (see QuantumNous#6947 for the full evidence):

  - 23 goroutines stuck in persistConn.roundTrip on a single 40h-old instance,
    blocked between 353 and 1894 minutes (5.9h to 31.5h)
  - 96.9% of the live heap, sampled after a forced GC, attributable to those three
    body copies (HeapAlloc 892 MiB surviving three GC cycles; HeapObjects dropping
    30x while bytes dropped only 25%)
  - the live floor grows with uptime: 33.7 MiB at 0.1h, 89.2 at 13.8h, 510.0 at 40.1h,
    955.2 at 146.8h, OOMKilled at 172.9h -- same image, same config, same load

Doubling the memory limit and adding GOMEMLIMIT only moved the OOM from 132h to 172.9h.

RELAY_TIMEOUT (http.Client.Timeout) cannot be used for this: it covers the whole response
read and would cut legitimate long streaming calls, which is why it defaults to 0.
ResponseHeaderTimeout only bounds the wait for the headers; streaming after they arrive is
unaffected.

The default is deliberately generous. Non-streaming upstreams usually send the response
headers only once generation has finished, so the value has to leave room for a long
completion. 1800s is 12x shorter than the shortest hang observed here while leaving
several times the headroom a normal non-streaming request needs; 0 restores the previous
unbounded behaviour.

The assignment goes next to the other transport.* lines rather than inside the else
branch: newRelayHTTPTransport() normally takes the http.DefaultTransport.Clone() path,
and DefaultTransport does not set ResponseHeaderTimeout either.

This repo already sets ResponseHeaderTimeout on its other outbound transports
(controller/model_sync.go, controller/ratio_sync.go); the relay path appears to have
been missed.

Refs QuantumNous#6947. Likely also the root cause of QuantumNous#6731, which reported the same symptom
(production OOM on /v1/responses after ~64h) but was closed for template reasons.

* review: clamp overflowing timeout values and switch the test to testify

Addresses the two CodeRabbit findings on this PR.

Overflow (common/init.go:113): a RELAY_RESPONSE_HEADER_TIMEOUT beyond ~9.2e9 seconds
overflows time.Duration and can wrap into a *tiny positive* timeout, which would cut
every relay request instead of only the stuck ones. The value is now clamped before the
conversion, with regression tests for both the negative and the overflowing input.

I did not add fail-on-startup validation for negative values, for two reasons: the
existing `if seconds > 0` guard already treats them as "disabled", and the neighbouring
env-driven timeouts in this file are less strict still -- RelayIdleConnTimeout is
converted with no guard at all. Failing startup on a bad value would be a behaviour
change out of step with the rest of the file; happy to add it if you'd prefer that
direction repo-wide.

Test style: switched to testify (require.Equal / require.Zero / require.Positive), which
is what every other test under service/ uses.

go build, go vet and go test ./common/... ./service/... pass.
(`go build ./...` fails on the `web/dist` embed both with and without this change -- the
frontend bundle is not checked in.)

* fix initialize database

* fix(model): drop leftover prefill_groups unique constraints before AutoMigrate (QuantumNous#7100)

* Revert "fix(model): drop leftover prefill_groups unique constraints before Au…" (QuantumNous#7101)

This reverts commit 69a41ee.

* fix(model): drop leftover prefill_groups unique constraints before AutoMigrate

---------

Co-authored-by: zcxads666 <128150298+zcxads666@users.noreply.github.com>
Co-authored-by: seefs001 <i@seefs.me>
Co-authored-by: Uladzislau <53997152+VladKabiak@users.noreply.github.com>
Co-authored-by: Calcium-Ion <i@caion.me>
Co-authored-by: Alex Xiang <ax2@zicode.com>
Co-authored-by: Seefs <40468931+seefs001@users.noreply.github.com>
Co-authored-by: 憧憬Licoy <licoycn@gmail.com>
Co-authored-by: PuppetKL <154485567+PuppetKL@users.noreply.github.com>
Co-authored-by: ruiyunzhao <91191418+CR-Yun@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Xayinn <129403670+LinineTy@users.noreply.github.com>
Co-authored-by: txgo <tianxi.liu@gmail.com>
yiranxiaohui pushed a commit to yiranxiaohui/new-api that referenced this pull request Sep 2, 2026
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.

2 participants