Skip to content

Test: cover enable_thinking precedence (top-level > kwarg > --thinking) at handler level #263

Open

Description

@solderzzc

Follow-up from the review of #260 (merged as 7376b6b), which fixed #259.

Gap. The precedence in handleChatCompletion (top-level enable_thinking, then chat_template_kwargs.enable_thinking, then --thinking) is inline in the handler. chatTemplateContext takes the already-decided Bool, so the four new unit tests in tests/SwiftLMTests/ChatRequestParsingTests.swift never see the precedence. tests/test-contract.sh test 17 only checks that mixed-type kwargs are accepted, and 17b checks the 400 body. Swapping the precedence branches leaves every test green.

Suggested test. Move the precedence decision into a small pure function (for example resolveThinking(topLevel:kwarg:config:)) and test each branch, including the conflicting case where the top-level value must win over the kwarg. Add one contract case with a top-level value that conflicts with the kwarg.

Severity: medium (no current bug; a regression guard).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions