Skip to content

feat(recipes): add qwen3.5-35b-a3b-dpo recipe (#276) - #615

Merged
MakazhanAlpamys merged 2 commits into
MakazhanAlpamys:mainfrom
Srinivasan8888:feature/recipe-qwen35-a3b-dpo
Sep 1, 2026
Merged

feat(recipes): add qwen3.5-35b-a3b-dpo recipe (#276)#615
MakazhanAlpamys merged 2 commits into
MakazhanAlpamys:mainfrom
Srinivasan8888:feature/recipe-qwen35-a3b-dpo

Conversation

@Srinivasan8888

Copy link
Copy Markdown
Contributor

Adds the ready-made qwen3.5-35b-a3b-dpo recipe. The catalog shipped the MoE SFT
sibling qwen3.5-35b-a3b-sft in v0.71.24 but no preference-tuning variant for the
same base.

Closes #276.

Overlap with #512 — read this first

#512 by @kok-o covers this issue and has done since 2026-08-23, eight days ahead
of me.
I did not see it before claiming: #276 carried no claim comment and #512's
overlap is visible only in its title. @MakazhanAlpamys has since asked @kok-o to
release #276 and told me to open this anyway, with @kok-o keeping the right to
reclaim it today. If they take it back, close this — they filed first and that
counts.

One correction to my own claim comment. I described the modality: text
contract as something I found. I found it independently, as a red test rather than
by reading — but it is not novel: #512 registered the same entry in
tests/test_issue427_qwen35_text_modality.py eight days before I started.

@kok-o hit that contract first. I would rather say so here than have a reviewer
find it by diffing the two branches.

The one substantive difference between the two implementations

Every DPO recipe in the catalog uses the same learning rate. Measured, not assumed:

$ python - <<'PY'
from soup_cli.recipes.catalog import RECIPES
from soup_cli.config.loader import load_config_from_string
for n, r in RECIPES.items():
    if r.task == "dpo":
        print(load_config_from_string(r.yaml_str).training.lr, n)
PY

5e-06   deepseek-r1-distill-llama-8b-dpo     5e-06   llama3.1-8b-dpo
5e-06   deepseek-r1-distill-qwen-1.5b-dpo    5e-06   llama4-scout-17b-dpo
5e-06   deepseek-r1-distill-qwen-7b-dpo      5e-06   mistral-7b-dpo
5e-06   gemma3-27b-dpo                       5e-06   pixtral-dpo
5e-06   glm-5.1-dpo                          5e-06   qwen2.5-7b-dpo

Unanimous, 10 of 10. This PR uses 5e-6, matching that and the issue's explicit
instruction to copy the qwen2.5-7b-dpo shape. #512 uses 1e-5, which would be
the only DPO recipe in the catalog off the family value. Stating it as a fact for
whoever reviews; it is not an argument about who should land.

Shape

The qwen2.5-7b-dpo DPO half — format: dpo, lr: 5e-6, dpo_beta: 0.1,
preference_train.jsonl — with the MoE settings from the qwen3.5-35b-a3b-sft
sibling: moe_lora: true, moe_aux_loss_coeff: 0.01, LoRA r16/a32, 4-bit,
grad-accum 8, max_length: 4096.

modality: text is taken from the SFT sibling rather than from the
qwen2.5-7b-dpo template, which has no modality line.
test_issue427_qwen35_text_modality.py holds every Qwen3.5/3.6/3.8 recipe to the
measured decoder-only decision instead of letting it fall back to the schema
default, and its family audit fails on any unregistered new sibling. A literal copy
of the template would have shipped a recipe escaping that contract.

Catalog count 159 → 160 across the three doc-count sites and the four catalog-size
checkpoints the sync guard enforces.

Mutations — five killed, one equivalent

Mutation Result
typo in the model id (Qwen3.5-35B-A3BQwen3.5-35B-A3) 2 fail
recipe entry deleted entirely 5 fail
MoE settings dropped (plain dense DPO) 1 fails
modality: text removed (falls back to schema default) family audit fails
format: dpoformat: auto 1 fails
dpo_beta: 0.1 dropped survives — equivalent

The survivor is not a coverage gap and I am deliberately not killing it. dpo_beta
defaults to 0.1 at src/soup_cli/config/schema.py:1237:

dpo_beta: float = Field(default=0.1, gt=0, description="DPO beta — KL penalty coefficient")

so removing the line produces a byte-identical SoupConfig — there is no behaviour
left to detect. Killing it would require asserting on the YAML source text rather
than on the loaded config, and a test that pins source strings instead of behaviour
is a shape this project has removed from its own suite before.

glm-5.1-dpo has the identical property. That makes it a repo-wide question about
whether explicitly-declared recipe values should be pinned against drift in schema
defaults — not something to answer asymmetrically inside this PR. Per
@MakazhanAlpamys on #276, it stays unaddressed here; happy to open it as its own
issue if it is wanted.

Verification

pytest tests/test_recipes.py tests/test_recipes_v031.py \
       tests/test_issue427_qwen35_text_modality.py tests/test_v07124.py \
       tests/test_v07130.py tests/test_v07132.py tests/test_config.py \
       tests/test_cli_startup_is_light.py      -> 1792 passed, 1 skipped
pytest tests/ -k "recipe"                      -> 1993 passed, 17466 deselected
ruff check src/soup_cli/ scripts/ tests/       -> All checks passed
git diff -- tests/ | grep -c "^-[^-]"          -> 5

All five removed test lines are the 159 → 160 count-bump sites themselves
(assert len(RECIPES) == 159 ×4 and the test_catalog_size_is_159 rename). No
test deleted, skipped or weakened.

Qwen/Qwen3.5-35B-A3B resolves on the Hub (/api/models/... → 200); it is also
the id the SFT sibling already ships. soup recipes show and soup recipes use
were both run live, not only asserted on.

The full default suite with the --cov-fail-under=77 gate is still running locally
at the time of opening; I will report the result in a comment rather than leave the
claim implied. Everything above is from runs that completed.

The catalog shipped the MoE SFT sibling qwen3.5-35b-a3b-sft in v0.71.24 but no
preference-tuning variant for the same base. Add qwen3.5-35b-a3b-dpo, pairing
the qwen2.5-7b-dpo DPO shape (format: dpo, lr 5e-6, dpo_beta 0.1,
preference_train.jsonl) with the sibling's MoE settings (moe_lora: true,
moe_aux_loss_coeff: 0.01, LoRA r16/a32, 4-bit, grad-accum 8), and bump the
recipe count 159 -> 160 across the three doc-count sites and the four
catalog-size checkpoints the sync guard enforces.

The entry carries an explicit `modality: text`, taken from the SFT sibling
rather than from the qwen2.5-7b-dpo template, which has no modality line:
test_issue427_qwen35_text_modality.py holds every Qwen3.5/3.6/3.8 recipe to the
measured decoder-only decision instead of letting it fall back to the schema
default, and its family audit is registered here so the new sibling joins that
contract rather than silently escaping it.

Adds TestIssue276Qwen35A3bDpoRecipe (mirrors the MakazhanAlpamys#280 MoE-DPO recipe-test
pattern) pinning the exact model id on both RecipeMeta.model and the parsed
config.base, the DPO half and the MoE half of the shape, and a check that the
DPO variant shares its base and size with the SFT sibling while differing in
task -- so a wrong model id or a half-copied shape is caught.

Closes MakazhanAlpamys#276

Claude-Session: https://claude.ai/code/session_01S16V9Zwx9TrSEWXwLtyD4z
@Srinivasan8888

Copy link
Copy Markdown
Contributor Author

Full default suite result, as promised in the description rather than left implied.

pytest tests/            -> 1 failed, 19351 passed, 102 skipped, 5 deselected (20:27)
Required test coverage of 77% reached. Total coverage: 82.70%

The one failure was mine, and it was an artefact of my own edit rather than of this branch. test_issue487_changelog_fragments.py::test_repository_fragments_match_the_newest_release read the changelog fragment while I was renaming it 583.added.md -> 615.added.md mid-run, once the PR number existed. The guard requires a fragment's body to reference its own filename number, so it caught the file in the window where the two disagreed — which is the guard working correctly, on a state that was never committed.

I reproduced it deliberately rather than assuming that was the cause:

$ # fragment named 583, body referencing 615
$ pytest tests/test_issue487_changelog_fragments.py::test_repository_fragments_match_the_newest_release
ChangelogError: changelog.d/0.73.3/583.added.md: fragment must reference #583
1 failed

Against the committed tree:

$ git diff HEAD --stat          # empty, working tree == pushed commit
$ pytest tests/test_issue487_changelog_fragments.py
17 passed

So: 19351 passed on the branch, the coverage gate cleared at 82.70% against a floor of 77%, and the single red cell does not reproduce on anything that exists in the PR. CI is the independent check on that claim and is running now — if it disagrees with me, believe CI.

@MakazhanAlpamys

Copy link
Copy Markdown
Owner

@Srinivasan8888 — reviewed. Every claim in your #276 comment holds when checked independently, and the PR is a merge on the substance. Two small things first, and both are cheap enough to land before CI even finishes its current run.

Your claims, verified rather than accepted

your claim checked
git diff -- tests/ | grep -c '^-[^-]' is 5, all count bumps exactly 5: one def test_catalog_size_is_159 rename + four assert len(RECIPES) == 159. No test deleted or weakened.
159 -> 160 across 3 doc sites + 4 catalog checkpoints no site missed; the only surviving 159 is the historical narrative line in the docstring, which is correct as-is
Qwen/Qwen3.5-35B-A3B resolves on the Hub HTTP 200
recipes show / use run live reproduced — the written soup.yaml reloads to Qwen/Qwen3.5-35B-A3B dpo text dpo 5e-06 0.1 True 0.01
ruff check clean All checks passed
tests/test_recipes.py test_recipes_v031.py test_issue427_qwen35_text_modality.py   1357 passed
tests/test_v07124.py test_v07130.py test_v07132.py test_issue487_...               412 passed, 1 skipped

The mutations — your five, plus three of mine

mutation killed by
typo in the model id TestIssue276Qwen35A3bDpoRecipe::test_recipe_loads_with_expected_dpo_moe_shape (+4)
catalog entry deleted 8 failures, incl. test_qwen35_family_audit_covers_every_current_recipe
MoE settings dropped test_recipe_loads_with_expected_dpo_moe_shape
modality: text removed test_qwen35_family_recipes_explicitly_select_language_tower[qwen3.5-35b-a3b-dpo]
format: dpo -> auto test_recipe_loads_with_expected_dpo_moe_shape
RecipeMeta.model alone -> A3C same node id (+4)
YAML base: alone -> A3C same, plus test_v07124::test_all_recipes_yaml_base_matches_model
unregister from the #427 audit set test_qwen35_family_audit_covers_every_current_recipe

Splitting the id mutation across its two surfaces is the one I care about: each is pinned independently, so a future edit to one that forgets the other cannot pass. That is precisely the guard #512 was asked for and did not have, and you had it before being asked.

The modality: text deviation was the right call and your reasoning was the right reasoning — you found it as a red test rather than by reading, and following the issue text literally would have shipped a recipe escaping a contract written for that family.

On dpo_beta: confirmed equivalent — stripping the line gives a.model_dump() == b.model_dump(). Reporting it beats manufacturing a kill for it. Nothing to add.

ANSI: not at risk, and I reproduced rather than assumed it. Under FORCE_COLOR=1 TERM=xterm-256color, recipes show output does contain \x1b, but Pygments emits the YAML scalar as a single token so no escape lands inside Qwen/Qwen3.5-35B-A3B. Eight sibling assertions use the same pattern with longer ids and are equally intact. Different from the Typer flag-name case that reddened CI four times.

Two things, both one line

1. The changelog fragment credits nobody. changelog.d/0.73.3/615.added.md needs (#276 by @Srinivasan8888 in #615) at the end of the entry. Our own checklist says to apply credit at merge time and it has been applied once in thirty merges — this release is themed on outside contributions and I would rather fix the habit on a PR where the credit is unambiguous.

2. Two test method names are now stale. test_v07130.py::TestRecipes::test_catalog_size_is_159 and test_v07132.py::TestAsrRecipes::test_catalog_size_is_159 both assert == 160 under a name saying 159. The recipe PR immediately before yours (596b828, #582 — also yours) renamed all three _158 -> _159; this one renamed only the test_recipes.py copy. Purely cosmetic, but it is your own convention and a name that contradicts its assertion is a small future trap.

(test_v07124.py::test_total_recipe_count_is_158 is already stale on main — pre-existing, not yours, leave it.)

Merge state

main has moved four commits past your base since you opened this — none touch the catalog or any count site, so no drift and no conflict.

Push those two and I merge on green. And thank you for holding while the #512 overlap was resolved instead of pushing ahead — that made the whole thing easier to settle fairly.

…MakazhanAlpamys#276)

Review follow-up on MakazhanAlpamys#615.

The fragment carried no credit. Applied in the house form the other 0.73.3
fragments use -- credit inline in the opening sentence, `(MakazhanAlpamys#276 by
@Srinivasan8888 in MakazhanAlpamys#615)`, rather than appended after `Closes MakazhanAlpamys#276`, which
would have repeated the issue number twice in one line.

test_v07130.py::TestRecipes and test_v07132.py::TestAsrRecipes both assert
`== 160` under a method named `test_catalog_size_is_159`. MakazhanAlpamys#582 renamed all
three _158 -> _159 copies; this branch had renamed only the test_recipes.py
one. A name that contradicts its own assertion is a trap for whoever bumps
the count next.

test_v07124.py::test_total_recipe_count_is_158 is left alone: it is stale on
main already and predates this branch.

Refs MakazhanAlpamys#276

Claude-Session: https://claude.ai/code/session_01S16V9Zwx9TrSEWXwLtyD4z
@Srinivasan8888

Copy link
Copy Markdown
Contributor Author

Both pushed in 8dca7255.

1. Credit. Added — but not in the position you specified, and I want to say why rather than quietly move it. Appending (#276 by @Srinivasan8888 in #615) after the existing Closes #276 would have put the issue number in the same line twice. The other 0.73.3 fragments carry the credit inline in the opening sentence instead:

468.added.md:  names (#459 by @blackcoderx in #468).**
490.changed.md: ... per-PR changelog fragments (#487 by @Amix29 in #490).**
604.security.md: ... octal IPv4 forms (#600 by @here-2007 in #604).**

So I restructured the fragment to that form — bolded lead sentence, credit inline, Closes #276 dropped as redundant with it. If you would rather have the literal appended form, say so and I will switch it; you set the checklist and I would rather match the house style than my reading of it.

While rewriting I also folded the modality: text contract into the fragment body. It was in the PR description and the commit message but not in the thing that ships to users, and it is the part of this change a future reader is most likely to need.

2. Stale names. test_v07130.py::TestRecipes and test_v07132.py::TestAsrRecipes renamed test_catalog_size_is_159_160. test_v07124.py::test_total_recipe_count_is_158 left untouched, as you said — it is stale on main and not mine to fix here.

pytest test_issue487_changelog_fragments.py test_v07130.py test_v07132.py test_recipes.py
  -> 245 passed, 1 skipped
ruff check tests/  -> All checks passed

Thank you for splitting the model-id mutation across both surfaces and re-running rather than reading the test names — the RecipeMeta.model alone vs base: alone split is a sharper guard than the one I wrote, and I will carry it forward.

On the merge-state note: rebasing onto the four new commits is a one-liner if you want the branch linear before merge — say the word, otherwise I will leave the history as-is.

@MakazhanAlpamys

MakazhanAlpamys commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Merged as 53e92c0. Thank you @Srinivasan8888 — and the thing worth naming is not the recipe.

What made this merge on the first review

You deviated from the issue text and said so before being asked. #276 says to copy qwen2.5-7b-dpo, which has no modality: line. tests/test_issue427_qwen35_text_modality.py holds every Qwen3.5/3.6/3.8 recipe to an explicit modality: text, and its family audit fails on an unregistered sibling — so a literal copy would have shipped a recipe escaping a contract written for exactly that family. You found it as a red test rather than by reading, took modality: text from the SFT sibling, and registered the recipe in the audit set.

The model id is pinned on two independent surfaces. Mutating RecipeMeta.model alone and mutating the YAML base: alone each fail a named test:

RecipeMeta.model -> A3C   TestIssue276Qwen35A3bDpoRecipe::test_recipe_loads_with_expected_dpo_moe_shape (+4)
YAML base:       -> A3C   same, plus test_v07124::test_all_recipes_yaml_base_matches_model

That is precisely the guard the competing PR for this issue lacked, and you had it before anyone asked for it.

You reported a surviving mutation rather than manufacturing a kill. Removing dpo_beta: 0.1 gives a byte-identical config — confirmed, a.model_dump() == b.model_dump(), schema.py:1237 defaults it to 0.1. Equivalent mutation, correctly identified, correctly left alone. A test written to kill it would have had to assert on YAML source text, which is the failure mode this project has removed from its own suite more than once.

Everything re-checked independently

your claim result
5 deleted test lines, all count bumps true — 10 across the whole diff, every one a 159->160 bump or a test-method rename
counts bumped at 3 doc sites + 4 catalog checkpoints no site missed
Qwen/Qwen3.5-35B-A3B resolves on the Hub HTTP 200
recipes show / use run live reproduced; written soup.yaml reloads correctly
recipe/config/CLI suites          1357 passed
v07124 + v07130 + v07132 + fragments   412 passed, 1 skipped
ruff                              clean
CI                                14/14

Eight mutations, eight killed — your five plus three of mine (lr 5e-6 -> 1e-5, the two id surfaces split apart, unregistering from the #427 audit set).

ANSI checked rather than assumed: under FORCE_COLOR=1 TERM=xterm-256color the output does carry escapes, but Pygments emits the YAML scalar as one token so none land inside the model id.

Two process notes

You held while the #512 overlap was resolved instead of pushing ahead, and you asked about the #379 overlap on #373 before writing code — citing this exact situation as the reason. That is the entire lesson of yesterday applied within hours of it happening, and it is worth more to this project than any single recipe.

The changelog credit line and the two stale test_catalog_size_is_159 names you fixed in two minutes: thank you. Crediting contributors is the maintainer's job and it had been applied once in thirty merges before this week.

Your 6th merge. #618, #619 and #620 are next in my queue.

MakazhanAlpamys pushed a commit that referenced this pull request Sep 1, 2026
Fills the gap left by the v0.71.24 model-family expansion, which shipped
kimi-k2.6-sft but no GRPO reasoning variant: grpo_beta 0.1, num_generations 4,
reward_fn accuracy, moe_lora, gradient_checkpointing, 4-bit, max_length 8192,
keeping the Modified MIT licence note in the description. Catalog 160 -> 161.

Six mutations, targeted at the new recipe block specifically, six killed by a
named test:

  RecipeMeta.model -> Kimi-K2.7   TestIssue281KimiK26GrpoRecipe::test_recipe_loads_with_exact_model_id
                                  + test_v07124::test_all_recipes_yaml_base_matches_model
  YAML base: -> Kimi-K2.7         ..::test_recipe_loads_with_exact_model_id
                                  + ..::test_show_and_use_recipe
  grpo_beta 0.1 -> 0.2            ..::test_recipe_loads_with_exact_model_id
  num_generations 4 -> 8          same
  reward_fn accuracy -> format    same
  task grpo -> sft                TestRecipeCatalog::test_recipe_tasks_match_yaml + the above

The two model-id surfaces are pinned INDEPENDENTLY -- RecipeMeta.model and the
YAML base: each fail on their own -- so an edit to one that forgets the other
cannot pass. That is the guard #512 was asked for and did not have, and this PR
carried it before being asked. The control mutation was re-run on the rebased
head rather than the reviewed one, since a rebase is a new tree.

Worth recording: the FIRST mutation run reported three of these as surviving.
That was a maintainer error, not a coverage gap -- a replace-first-occurrence kept
hitting the pre-existing kimi-k2.6-sft entry instead of the new block. Re-run
line-targeted, everything dies. Published in the PR thread rather than only the
corrected run.

The rebase was needed because #615 (qwen3.5-35b-a3b-dpo, #276) merged first and
both PRs bump the same catalog counter and append a test class at the same point.
That is merge ordering, not anything about this work: whichever landed second was
going to hit it. Verified after the author's rebase that nothing was lost --
comparing test methods by AST against main, the only absences are the intended
test_catalog_size_is_160 -> _161 renames, and both test classes are present with
all four of their methods.

  merge-tree against main   0 conflicts
  CI                        14/14
  ruff                      clean

The changelog fragment's credit line is corrected in a follow-up commit rather
than by asking the author to push again; crediting the contributor is the
maintainer's job.

Closes #281
MakazhanAlpamys pushed a commit that referenced this pull request Sep 1, 2026
Refs #275. Adds the `qwen3.5-9b-dpo` recipe, completing the sft/grpo/dpo trio for
that base. Catalog 161 -> 162.

Geometry is the SFT sibling's plus the DPO half of `qwen2.5-7b-dpo`: lr 5e-6,
dpo_beta 0.1, LoRA r16/a32, 4bit, max_length 4096. All eleven pre-existing DPO
recipes are lr=5e-06 / dpo_beta=0.1, so the new one is not an outlier. The model
id `Qwen/Qwen3.5-9B` is byte-identical to the two shipped siblings.

Seven mutations, seven killed, no survivors. The two that decide the merge are the
same pair that decided #615:

  RecipeMeta.model changed, YAML base: untouched -> 4 failed
  YAML base: changed, RecipeMeta.model untouched -> 3 failed

so an edit that repairs one surface and forgets the other cannot pass. Also killed:
dropping `modality: text` (fails the Qwen3.5 family audit by recipe name), reverting
either of two independent count sites, `format: dpo` -> `auto`, deleting the entry
(11 tests), and keeping the recipe while UNREGISTERING it from
`EXPECTED_QWEN35_TEXT_RECIPES` — the last one is the direction that matters for the
family contract, since deleting the recipe only proves the easy half.

Count synchronisation is complete: `tests/test_recipe_count_is_synced.py` declares
five documentation sites and all five are updated; a repo-wide search for the old
count returns nothing. The 7 deleted test lines are four count-assertion bumps and
three test renames — no assertion weakened.

A changelog fragment was missing and is added in a follow-up maintainer commit
rather than sent back for another round.
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