Latest Results
fix: port embedding fine-tune stage 3 to SentenceTransformerTrainer (#2828)
Closes #2807
## The defect
Stage 3 has never been able to run, on any pinned version.
`contrastive_fine_tune` read two attributes off the bare
`sentence_transformers` module:
- `st.losses.MultipleNegativesRankingLoss`
- `st.datasets.NoDuplicatesDataLoader`
Neither is bound. Both subpackages live at
`sentence_transformers.sentence_transformer.losses` / `.datasets`, and
the package `__init__` imports *names* out of them rather than the
modules. Upstream's compatibility shim for the old paths is a
`sys.meta_path` **finder**, which fires on an `import` statement and
never on attribute access. Both reads raise `AttributeError`.
Nothing caught it because no test anywhere imported the real package:
the unit tier substitutes a `SimpleNamespace` carrying a single
`SentenceTransformer` attribute, CI never installs the optional extra,
and the two existing `TestContrastiveFineTune` tests both return on
input validation before any import. The loss, the dataloader, the
training-example builder, the progress hook and the `model.fit(...)`
call had zero coverage.
**A second, independent blocker** was found while implementing: even the
legacy `fit()` path in v6 raises on `is_datasets_available()` and
constructs a `SentenceTransformerTrainer` internally. `datasets` and
`accelerate` are not dependencies of `sentence-transformers` (they sit
in its `train` extra), and our extras pinned the bare package, so stage
3 would have failed on a missing `datasets` even with the attribute
reads fixed. Both preflight probes only imported the package, so they
reported ready and the run died hours later, two stages in.
## The port
Every training symbol is resolved by **module path**, in one new adapter
(`memory/embedding/fine_tune_trainer.py`, `# module-kind: adapter`), so
the unreachable attribute reads disappear rather than being patched
around. `InputExample` becomes a `datasets.Dataset`;
`NoDuplicatesDataLoader` becomes `BatchSamplers.NO_DUPLICATES`;
`model.fit(...)` becomes `SentenceTransformerTrainer` +
`SentenceTransformerTrainingArguments`; progress and cancellation move
onto a `TrainerCallback`.
Progress now reads the trainer's own `global_step / max_steps` rather
than a hand counter, so `0.0..1.0` monotonicity is structural.
Cancellation is checked on **every** step and on `on_train_begin`: the
old interval was a faithful port of a latent bug, since a run with fewer
steps than the interval never reaches a multiple of it and cannot be
cancelled at all (a 12-row epoch is three steps).
**Ragged negatives.** Stage 2 emits between zero and `top_k` hard
negatives per query, because its similarity margin can starve a row, and
a training dataset is columnar, so rows of differing width cannot share
a table. Rows are bucketed by negative count into a multi-dataset
training set, all buckets sharing one loss and drawn from
proportionally, so a batch always has a uniform column count. Every row
trains, at its own hardness, and no mined negative is discarded.
## Security
Both found by running the ported path for real, and both were
unreachable before it because stage 3 could never run.
**Training text shipped in the checkpoint.** The trainer attaches a
model-card callback in its own constructor. It samples the training rows
into "widget examples" and renders them verbatim into `README.md`: a
marker-string run put all three markers into a 16 KB card, and those
rows are excerpts of the organisation's own documents in a file built to
travel with the checkpoint. The callback is now removed through the
override upstream documents for the purpose, and `save_checkpoint`
passes `create_model_card=False` as the second line.
**Outbound Hub call for a local base model.** The same callback asks
huggingface.co about the base model, and so does every
`SentenceTransformer(...)` load. For a base model that is a local
checkpoint path that sends the path of a private on-disk model to a
third party to answer a question with no answer, and on a host with no
egress it makes every load wait for the request to fail.
`load_base_model` derives `local_files_only` from whether the reference
names a directory, so a hub identifier still resolves and downloads
normally.
## Other fixes in this PR
- **Fail loud on damaged stage 2 output.** Empty triples, a missing
`query`/`positive`, or a non-list `negatives` are refused naming the
record index, instead of being coerced into a checkpoint that only looks
wrong hours later as a weak score at the promotion gate.
- **Dependency guards catch what actually breaks.** `ImportError` alone
missed a lazily-resolved submodule re-raising as `RuntimeError` and a
native extension failing as `OSError`, both of which escaped untyped
past a module that documents "every symbol or
`FineTuneDependencyError`". The warning now records which module broke
and how, rather than one indistinguishable line for five distinct
failures.
- **Both probes ask the trainer question.** A deployment missing
`datasets`/`accelerate` now fails at preflight rather than mid-run,
through one shared `verify_fine_tune_dependencies` instead of two copies
of the same rationale. The preflight request ceiling was sized for the
directory walk only; it now covers the in-process probe's own cold
imports, which otherwise 503 a healthy deployment on first use.
- **Cancellation names the stage it interrupted.** `check(stage=...)` is
mandatory: abandoning a corpus scan and abandoning hours of GPU time are
not the same loss, and one shared label could not tell them apart.
- **Shutdown reaches an in-flight run.** Nothing in the teardown touched
the fine-tune orchestrator, so SIGTERM ran on to the SIGKILL and took
the unwritten checkpoint with it.
- **Terminal progress is never throttled**, so a stage finishing inside
the throttle window still reports done; the emit bookkeeping is now
locked rather than relying on an undocumented single-caller invariant.
- **`top_k` is bounded** (each negative is its own Arrow column and its
own bucket), an `ERROR:` line from the container is truncated before it
reaches an operator's failure message (the prefix is a convention on a
stream every library in the container can write to), and the
checkpoint-deploy event no longer reports "config not updated" and
"fully applied" under one name.
## Packaging
All four fine-tune declarations pin `sentence-transformers[train]` plus
`datasets` and `transformers` directly, since the adapter imports both
by name. The image label gains the new components and `BSD-2-Clause`.
## Tests
The extra-free half runs everywhere: bucketing, warmup arithmetic, the
progress and cancellation logic, the trainer assembly,
`contrastive_fine_tune`'s happy path, and the dependency-failure branch
(driven by a meta-path finder, so the failure shapes CI cannot otherwise
produce are still covered).
The guarded half imports the **real** package, because a stand-in agrees
with whatever signature it is handed and only a real import can observe
a symbol that no longer exists. A new `test-fine-tune-extra` CI job
installs `fine-tune-cpu` and runs it;
`SYNTHORG_REQUIRE_FINE_TUNE_EXTRA=1` turns the availability guard from
skip into failure, and the job asserts the test report shows zero skips,
because pytest exits 0 on a fully-skipped file. It also carries the
retry ladder, core dumps and `pip-audit` its siblings have, none of
which covered the extra's dependency tree before.
The existing `SimpleNamespace`-based tests are kept; this **adds**
real-import coverage rather than making the fast tier need the extra.
## Verification
- Full unit tier: **45,853 passed, 53 skipped**
- Guarded real-import tests with the extra installed: **all ran, none
skipped**
- mypy strict clean, both with and without the extra installed
- All 88 pre-push gates green, 2m27s
- **Real end-to-end run** against `all-MiniLM-L6-v2`: bucketed 4/4/4,
trained, progress monotonic to 1.0, checkpoint written, **no `README.md`
and no training text anywhere in it**, no widget sampling, and a
cancelled run raises naming its stage refactor: remove the dynamic auto-scaling pipeline (#2827)
Part of #2823 (PR 1 of 2: scaling. The personality half follows in its
own PR, so this one deliberately does not close the issue).
Deletes `src/synthorg/hr/scaling/` and everything downstream of it: 31
modules, 4,781 LOC, plus its controller, subsystem, settings, events,
MCP tools and dashboard page.
## Why it goes
The audit verdict is on the issue. In short:
- `evaluate()` had exactly two callers, the REST route and the MCP tool,
both behind `hr.scaling_enabled`, which ships off.
- The threshold and composite triggers were **structurally
unreachable**: no background loop polled `should_trigger`, and
`update_signal` had zero producers anywhere in the tree, so no threshold
could ever observe a crossing.
- Nine dogfood rounds and the depth-3 recursion sweep never produced a
single scaling decision.
The gate-role hire path is **untouched and verified by name**:
`engine/review_staffing/reconciler.py` -> `hiring_pass.ensure_hire_open`
/ `finish_approved_hires` -> `hr/hiring_service.HiringService`. None of
them import anything from `hr/scaling/`; the dependency ran the other
way, so this removes a consumer rather than a provider.
`OffboardingService` is independently constructed by
`pruning_wiring.py:107` and does not orphan.
`tests/unit/hr/test_roster_growth_single_owner.py` is new and asserts
that going forward, at the four layers a second roster-grower could
reappear at: the package tree, the `hr` settings namespace, the HR
feature's controllers, and the MCP tool surface. It carries a positive
control so the suite cannot pass by having deleted the real path too.
## Migrations
Scaling owned no tables. That is not the same as having written nothing,
and it wrote into **three** tables it did not own, all of which outlive
it:
1. **`hiring_requests.payload`** carried `agent_delegate`.
`HiringRequest` is `extra="forbid"`, so a stale key fails validation on
read, and `_query_rows` turns one bad row into a `QueryError` for the
whole page, which is the page the staffing sweep reads. Worse, that row
still holds its `idx_hiring_requests_one_open_per_role` slot, so the
role it blocks can never be reopened by anything.
The guard matters here. SQLite's `json_extract` returns SQL NULL for a
key holding JSON null exactly as it does for an absent key, and the
writer calls `model_dump` without `exclude_none`, so the null spelling
is what nearly every stored row carries. A `WHERE ... IS NOT NULL` guard
therefore skips precisely the rows the migration exists to repair, which
is why the SQLite arm runs unguarded (`json_remove` is already a no-op
on an absent key). Postgres keeps its guard because `?` tests key
existence correctly. The asymmetry is deliberate and commented on both
sides.
2. **`approvals`** held PENDING `scaling:hire` / `scaling:prune` rows
raised by the deleted approval gate. Nothing left can act on one: the
orphan sweep keys on `task_id` and these carry none, and delete-time
retirement fires on a row being removed rather than a subsystem. They
are set to `expired`, with `decided_at` / `decided_by` left NULL, for
the reason `_approval_retire.py` already gives: a rejection is a
reviewer's verdict, and nobody made one.
3. **`custom_rules`** held any rule an operator built on
`scaling.total_decisions` or `scaling.success_rate`. `metric_path` is a
plain column with no vocabulary CHECK, so such a row survives the
database and fails in Python, where a field validator rejects a path the
registry no longer holds. One is enough to break every rule in the
deployment: the repository builds its page with
`tuple(_row_to_definition(row) for row in rows)` outside the
driver-error handler, so the raise reaches `GET /meta/custom-rules` and
the whole listing goes with it. Deleted rather than disabled, since an
unfiltered list still reads and validates a disabled row, and rewriting
the path would silently change what the operator's rule means.
`tests/unit/persistence/test_hiring_request_migration.py` covers all
three, seeding every payload shape (key with JSON null, key with a
value, key absent) plus a control row per table to prove each sweep is
bounded by its prefix rather than clearing the table.
## Also in this branch
Found while reviewing, fixed here rather than deferred: a signals
subsystem that declined by returning instead of raising
`SubsystemDeclinedError` (so `GET /subsystems` had nothing to report); a
controller test asserting a `scaling` domain that no longer exists, off
a synthetic mock, passing vacuously; three files the deletion pass
missed; eleven stale counts and entity references across docs, generated
data and the endpoint-table generator; and a handful of unlogged error
paths and dead logger bindings in the touched modules.
## Verification
All 88 pre-push convention gates green. Type-check clean. Affected unit
suites pass (5,012 locally, the full tier deferred to CI by design at
807 affected files). Both backends apply the new revision cleanly, and
`check_schema_drift_revisions.py` passes for each. Latest Branches
0%
release-please--branches--main--components--synthorg 0%
fix/fine-tune-sentence-transformer-trainer 0%
refactor/audit-hr-scaling-personality Ā© 2026 CodSpeed Technology