Merge pipeline config overlays by model ID#2290
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
@hogeheer499-commits please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
Reuse matching decoder and vision pipeline entries when applying config overlays, preserving filenames and existing session options. Unknown IDs continue to append in order. Reset active parser wrappers before vector growth and add a model-free regression test covering targeted updates and new IDs. Fixes microsoft#2004
47d288a to
95da9e4
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes Config::overlay() pipeline overlay semantics so decoder/vision pipeline entries are merged by model_id instead of always appended, preventing duplicate entries with empty filename that can break model loading (Issue #2004).
Changes:
- Merge pipeline overlays by reusing an existing pipeline entry when
model_idmatches; only append for new IDs. - Preserve existing pipeline component
filenameandsession_optionson partial overlays (while keepingrun_optionsreplacement behavior unchanged). - Avoid potential invalidation by resetting the active per-entry JSON parser element before appending to the pipeline vector.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/config.cpp |
Updates JSON overlay parsing to merge decoder/vision pipeline models by model_id and preserve existing per-model session_options on partial overlays. |
test/config_tests.cpp |
Adds a regression test asserting correct merge-by-model_id behavior for both decoder and vision pipelines. |
Description
Fixes #2004.
Config::overlay()currently appends decoder and vision pipeline entries even when an entry with the samemodel_idalready exists. A partial component overlay can therefore create a duplicate entry with an emptyfilename, causing model loading to fail.This change:
session_optionswhile applying a partial overlay;The existing replacement behavior for
run_optionsis unchanged.Testing
ConfigTests.PipelineOverlayMergesByModelId, covering:main(four entries instead of three) and passes with this change.RelWithDebInfotargets and ran the full CTest suite: 100% passed.clang-format, lintrunner, andgit diff --checkpass.AI assistance
OpenAI Codex (GPT-5) was used to assist with investigation, implementation, review, and testing. The resulting behavior was validated against current
mainwith both unit and end-to-end tests.