Skip to content

[WIP] refactor: finalize db_case_config at a single choke point - #872

Open
jamesgao-jpg wants to merge 1 commit into
zilliztech:mainfrom
jamesgao-jpg:refactor/db-case-config-finalize
Open

jamesgao-jpg wants to merge 1 commit into
zilliztech:mainfrom
jamesgao-jpg:refactor/db-case-config-finalize

Conversation

@jamesgao-jpg

Copy link
Copy Markdown
Collaborator

What

Unifies every post-construction modification of db_case_config into a
single choke point, so a case config is produced in exactly one place and
only read afterwards.

New vectordb_bench/backend/db_case_config.py:

  • finalize_db_case_config(db, case_type, base_config, *, parameters, dataset)
    composes the two existing post-construction transforms:
    1. CLI-only FTS routing (moved from select_cli_db_case_config in
      cli/cli.py: vector config → backend FTS config with compatible-field
      copy, plus BM25 overrides);
    2. dataset-derived metric_type (previously applied by mutating
      task.db_case_config in assembler.py), now via model_copy so the
      input config is never mutated.
  • Both task entry points call it: cli.run() (with the CLI parameters dict)
    and the web UI generate_tasks() (without). The assembler no longer
    mutates the config; select_cli_db_case_config remains as a thin
    backward-compatible wrapper.

Why

db_case_config was touched in four layers (CLI construction, run()
FTS routing, assembler in-place mutation, task-runner/client reads), and the
CLI-only routing was skipped entirely on the web UI path. This made changes
to config handling easy to break and the two entry points could diverge.
Routing both entry points through one resolver (and deleting the in-place
mutation) removes that class of bug.

Behavior / compatibility

  • No behavior change: FTS-routing semantics, the metric_type default rule,
    and both entry-point results are preserved. Covered by the new resolver
    tests plus the existing FTS-routing tests, which now exercise the same
    code through the compatibility wrapper.
  • Cross-PR note: this refactor moves the FTS-compatible field whitelist
    (including force_merge_target_size_mb) into the resolver, so it subsumes
    the one-line whitelist fix in PR feat: add CLI control for Milvus force-merge size and toggle #871 (cli/cli.py). When both merge, drop
    feat: add CLI control for Milvus force-merge size and toggle #871's cli/cli.py hunk — the field already lives in the resolver.

Testing (Level 2)

  • Affected component tests on the remote client worktree (origin/main base):
    tests/test_db_case_config.py (8 new), test_fts_cli_user_control.py,
    test_oss_opensearch_fts.py, test_milvus.py, test_milvus_zilliz_cli.py
    → 53 passed, 1 integration deselected.
  • Whole-repo ruff check → All checks passed; black --check → 230
    files unchanged (both match the main baseline).

Follow-up (separate PR)

frozen=True enforcement across all case-config classes (~55 root classes in
44 backend config files), reworking the self-assigning cached-field methods
in aws_opensearch/oss_opensearch parse_metric(), hologres, and
pgvector, with a cross-backend regression sweep.

All post-construction transformations of case configs now flow through
finalize_db_case_config() in vectordb_bench/backend/db_case_config.py:

- CLI-only FTS routing (previously select_cli_db_case_config in
  cli/cli.py) and the dataset-derived metric_type default (previously
  applied by mutating task.db_case_config in assembler.py) are composed
  in one resolver.
- Both task entry points call it: cli.run() (with the CLI parameters
  dict) and the web UI generate_tasks() (without).
- The assembler no longer mutates the config; consumers treat the
  finalized config as read-only.
- select_cli_db_case_config remains as a thin backward-compatible
  wrapper for existing callers and tests.

No behavior change: FTS-routing semantics, the metric_type default rule,
and both entry-point results are preserved (covered by the new resolver
tests plus the existing FTS-routing tests).

Note: this refactor moves the FTS-compatible field whitelist (including
force_merge_target_size_mb) into the resolver, so it subsumes the one-line
whitelist fix carried by PR zilliztech#871's cli/cli.py.

Signed-off-by: jamesgao-jpg <james.gao@zilliz.com>
@sre-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jamesgao-jpg
To complete the pull request process, please assign xuanyang-cn after the PR has been reviewed.
You can assign the PR to them by writing /assign @xuanyang-cn in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jamesgao-jpg jamesgao-jpg changed the title refactor: finalize db_case_config at a single choke point [WIP] refactor: finalize db_case_config at a single choke point Sep 15, 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