Skip to content

test(openai): cover LlamaCpp and LM Studio model-list parsing - #3878

Draft
amitksingh1490 wants to merge 3 commits into
mainfrom
fix/3507-local-model-parsing
Draft

amitksingh1490 wants to merge 3 commits into
mainfrom
fix/3507-local-model-parsing

Conversation

@amitksingh1490

@amitksingh1490 amitksingh1490 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Current-head warning — autofix scope drift

Follow-up, 04:26 UTC: the bot pushed a second commit c92980301 (attempt 2/3), an additional unrelated formatting change in forge_services/src/fd.rs. Current cumulative bot drift remains 139 files (+1205/-833 versus authored commit). CI passed on the previous bot head 8b43e0790, but autofix changed the head again; latest-head CI 35055605294 and autofix 35055605251 are queued/running, so the prior green CI is not current-head verification. Saved autofix-second.patch and autofix-current-head.bundle. No llama-server or lms executable was found on PATH, and neither default loopback port 8080 nor 1234 is listening. Real-provider and scope-decision gates remain unresolved.

Do not treat the current remote head as the locally verified change. Authored/tested commit da7f35110 changes one test file. At 2026-09-16 04:23 UTC, autofix-ci[bot] pushed 8b43e0790, changing 139 files (+1206/-835 relative to the authored commit) with widespread unrelated comment/formatting churn. The new fixtures/tests themselves are untouched; the same file also has unrelated pre-existing comments rewrapped. Saved autofix.patch and autofix-head.bundle alongside the original one-file changes.patch/3507.bundle.

Original-head autofix run https://github.com/tailcallhq/forgecode/actions/runs/35055104595 passed Cargo Fmt, Cargo Clippy and String Safety, then failed its Autofix step while publishing the bot commit. New-head CI https://github.com/tailcallhq/forgecode/actions/runs/35055372245 and autofix https://github.com/tailcallhq/forgecode/actions/runs/35055372246 are pending/running at this check. Original-head CI 35055104613 was subsequently cancelled after the bot push (its zsh performance job passed); it cannot establish new-head readiness. Release Drafter is not validation.

Blocked on a maintainer decision about removing/separating the unrelated bot churn without an endless revert/autofix cycle, then validation of the resulting head. No workflow was approved, no automation edited, and no force push/revert loop attempted. Local worktree deliberately remains at the clean, tested authored commit; remote bot diff reviewed but not executed locally.

Summary

Regression coverage and verification for #3507. Draft: not PR Ready.

The actual production parsing fix is already on current main: commit 9144a7200 (OrcaRouter work, commit message references #3623) made Architecture.modality and Architecture.tokenizer optional. Both the Chat Completions repository used by LlamaCpp and the Responses repository now used by LM Studio deserialize through this DTO. This PR deliberately does not duplicate that production change or claim to newly implement it.

  • Add same-file realistic LlamaCpp and LM Studio model-list fixtures, reduced from the original contributor's responses and omitting local filesystem paths/server arguments.
  • Assert full domain-model conversion, including image modalities and unknown optional capabilities.
  • Cover ordinary OpenAI /v1/models, absent/null/empty/populated architecture metadata, and continued rejection of missing model IDs.
  • Authored scope: one Rust test file only; no automation, configuration, snapshot, or unrelated formatting changes. Remote bot-added scope differs; see warning above.

Prior work and attribution

Reviewed #3508 first, including its diff, comments, approval and inactivity closure; it is closed, not merged. Thanks to Gergely Imreh (@imrehg) for the original diagnosis, optional-field fix, provider responses and manual provider verification in that PR, and to its other listed contributor Claude Opus 4.8. This regression coverage adapts those response shapes; the original credit is also retained beside the fixtures. Searches for #3507/#3508 and other open model/local-provider PRs found no open duplicate. The existing fix on main means reopening/cherry-picking the old production diff is unnecessary.

Validation on authored commit da7f351 (main + test-only change; NOT the bot-expanded head)

Worktree: /home/forge/worktrees/3507; durable evidence/patch/bundle: /home/forge/issue-patches/3507.
Build environment: Rust 1.97, debug profile, debug symbols/incremental disabled to fit available disk; reused the inactive 3858 Cargo cache (Cargo rebuilt this worktree), no release build. Repro environment recorded in verification-env.sh.

  • cargo test --locked -p forge_app: 727 passed, plus 1 doctest passed / 2 ignored. Repeated successfully after the negative control was removed.
  • cargo insta test --accept --test-runner cargo-test -p forge_app: passed; no snapshots to review/change.
  • cargo clippy --locked -p forge_app --all-targets -- -D warnings: passed.
  • cargo build --locked -p forge_main: passed (debug).
  • rustfmt --check --edition 2024 crates/forge_app/src/dto/openai/model.rs and git diff --check: passed.
  • Negative control: temporarily required the architecture fields via Serde field deserializers. cargo test --locked -p forge_app test_llamacpp_model_list failed with missing field modality, matching the report. Removed the mutation and reran the full crate suite successfully.
  • Safe local mock: python3 /home/forge/issue-patches/3507/cli-verify.py /home/forge/forge-3858-build/target/debug/forge passed for both built-in llama_cpp and lm_studio. Inspected --help, provider login --help, and config set model --help first. Automated provider login (same handler as /login) through local URL/API-key-skip/model-picker prompts, checked saved provider/model, ran config set model, and ran -p 'Reply LOCAL_MODEL_PARSING_OK only; do not use tools.'. Every final command exited 0 without a printed error and returned the marker. Request logs confirm /v1/models, LlamaCpp /v1/chat/completions, and LM Studio /v1/responses.
  • Mock isolation: fresh HOME, FORGE_CONFIG, working directory, environment allowlist, synthetic local credentials, telemetry/updates disabled, loopback servers only. No user's real config, credentials or conversations were changed. Initial harness expected API order instead of the UI's alphabetical model order; corrected the harness, then both providers passed.

Remaining readiness gates

  • Real provider verification is NOT done. No running LlamaCpp/LM Studio server or model was available here. All CLI evidence above is local fixture/mock verification, not real inference or authentication. Please provide a safely reachable test server/model, or run login/model selection and a harmless prompt on the target installations and share sanitized results. The contributor separately reported template issues; this PR does not claim to solve provider/model template compatibility.
  • Workspace cargo fmt --all -- --check fails on existing trailing whitespace in crates/forge_main/src/info.rs:75 (reproduced against the unchanged main file); changed-file format check passes. Stable Rust also warns that repository nightly-only formatting options are unavailable. No unrelated formatter cleanup included in the authored commit.
  • Build/test output notes an existing future-incompatibility warning in proc-macro-error2.
  • Latest-head CI/autofix was inspected and exposed the scope drift above; new-head CI is not yet complete, and the bot-expanded head has not been locally revalidated. The board stays Blocked / Needs Input until scope, CI and real-provider gates are satisfied, not PR Ready.

Related to #3507; no automatic issue closure requested.

Co-Authored-By: Gergely Imreh gergely@imreh.net
Co-Authored-By: ForgeCode noreply@forgecode.dev

Co-authored-by: ForgeCode <noreply@forgecode.dev>
@amitksingh1490 amitksingh1490 changed the title test: cover local provider model list parsing test(openai): cover LlamaCpp and LM Studio model-list parsing Sep 16, 2026
@amitksingh1490

Copy link
Copy Markdown
Contributor Author

Current-head readiness warning: autofix-ci[bot] pushed 8b43e07, expanding the authored one-file regression commit da7f351 to 139 changed files. The local 727-test/lint/debug/mock evidence applies to da7f351, NOT the expanded bot head. Original-head autofix passed both Clippy steps but failed its Autofix step while pushing; original CI was cancelled after the push. New-head CI/autofix remain running as of 04:24 UTC. Both original and bot diffs/bundles are saved. Please resolve how to separate/remove unrelated bot formatting without a revert/autofix loop, then validate the resulting head. No automation edits, workflow approval, force push, merge or issue closure performed. Real LlamaCpp/LM Studio verification also remains required; mock evidence is not live-provider evidence. Board stays Blocked, not PR Ready.

Co-Authored-By: ForgeCode noreply@forgecode.dev

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.

1 participant