This is a nightmare to review and rebase until merged though, also you seem to have pushed the same changes to your Qwen3-Next PR?

@CISC Ye, need them for working there, but I'll revert once I'm done (unless this is done first and I can merge on top).

I know it's a nightmare, I already kind of went through it when I asked an LLM to automate some tasks and it proceeded merrily ripping out methods just because some classes didn't inherit from llm_graph_context :>

If you want, I can write a script that runs tree-sitter on the original definitions in llama-model.cpp vs the new classes and shows any differences to verify that nothing was accidentally lost.

Maybe it would be easier to refactor that partially, just a subset of the models?

This seems to be a good change, just have some other ideas:

1. I think for now all models can share one single .h file. The main .cpp implementation can be split into smaller files as proposed here
2. Our naming convention uses - instead of _. Maybe just src/models/(model-name).cpp is enough, no need the llm_build prefix. For example: src/models/gpt2.cpp

All right, I've done the changes suggested by @ngxson - renamed the files to just .cpp and merged all the header files into one models.h. I've also merged all the current changes from master.

Could we possibly move this forward somehow? This isn't going to get any easier any time soon... @ggerganov ?

Yes, thanks for the help. I will take a look soon at the Qwen 3 Next PR and also this reorganization here. Likely over the weekend.

The diff is too big so I had a look at some files under your fork instead.

It looks good overall, just a nitpick stuff: Seems like many files starts with this pattern:

(maybe one empty line)
#include "models.h"
(some empty line)
...

While it's cleaner to be:

#include "models.h"
(one empty line)
...

Should be a real quick regex replace to change

@ngxson aye, done.

@pwilkin
There are 117 changed files in this PR.
You need to rebase this PR to show clear code change.

Thank you!

@pwilkin There are 117 changed files in this PR. You need to rebase this PR to show clear code change.

Thank you!

Sorry, I don't understand.

Yeah, there are 117 changed files in this PR because it's a refactoring - it extracts the builders from llama-model.cpp to separate files.

I don't think the exact architecture of the PR matters - GitHub will always show the final changes in the PR.

No, you have changes that don't belong here, from PRs that have been merged recently. This happens occasionally, not sure what exactly causes it. Rebasing your branch should fix it.

No, you have changes that don't belong here, from PRs that have been merged recently. This happens occasionally, not sure what exactly causes it. Rebasing your branch should fix it.

Oh, that. Yeah, that's some weird byproduct of merging the current master. I'll try to rebase instead.

@NeoZhangJianyu @0cc4m did a clean rebase

merging the current master

My recommendation would be always rebase and/or cherry-pick instead of merge.

Should be OK to merge this now?

Should be OK to merge this now?

There are some whitespace inconsistencies (f.ex. in model.h, most prototypes are clustered together, but some are separated), also not sure I like the new formatting of the build_xxx parameters, the old clusterings were easier to parse by sight.