Skip to content

Conversation

@samchon
Copy link
Owner

@samchon samchon commented Dec 18, 2025

This pull request makes a targeted update to the logic for separating parameters in the HttpLlmApplicationComposer. The change replaces the use of LlmSchemaComposer.separateParameters with LlmSchemaComposer.separate, likely reflecting a refactor or simplification in the schema composition utilities.

  • Refactored parameter separation in HttpLlmApplicationComposer to use LlmSchemaComposer.separate instead of separateParameters, aligning with updated schema composition methods.

@samchon samchon self-assigned this Dec 18, 2025
Copilot AI review requested due to automatic review settings December 18, 2025 16:55
@samchon samchon added the enhancement New feature or request label Dec 18, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a "universal" implementation of LlmSchemaComposer, replacing the previous model-specific dispatcher pattern with a unified implementation. The changes include a complete rewrite of LlmSchemaComposer.ts (from ~97 lines to 789 lines) and updates the API from model-based curried functions to direct property-based functions.

  • Complete rewrite of LlmSchemaComposer.ts with universal implementations of parameters, schema, separate, and invert functions
  • Attempted update of HttpLlmApplicationComposer.ts to use the new separate method instead of separateParameters
  • API change from method(model)({props}) pattern to method({props}) pattern

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/composers/LlmSchemaComposer.ts Complete rewrite implementing universal schema composition logic, replacing model-specific dispatcher with direct implementations of parameters, schema, separate, and invert functions
src/composers/HttpLlmApplicationComposer.ts Attempts to update method call from separateParameters to separate, but incorrectly retains the old API pattern of passing model as parameter
Comments suppressed due to low confidence (4)

src/composers/HttpLlmApplicationComposer.ts:237

  • The call to LlmSchemaComposer.separate is incorrect. The new separate function (lines 364-427 in LlmSchemaComposer.ts) takes a props object directly, not a model parameter that returns a function. This code attempts to call separate(props.model) which would pass the model as the props object, and then tries to invoke the result as a function, which will fail.

The call should be changed to:

LlmSchemaComposer.separate({
  predicate: props.config.separate as any,
  parameters: llmParameters.value satisfies ILlmSchema.ModelParameters[Model] as any,
  equals: props.config.equals ?? false,
})

Note: This same issue exists for LlmSchemaComposer.parameters (line 179) and LlmSchemaComposer.schema (line 189) which also changed their API to accept props directly instead of a model parameter, but were not updated in this PR.

        ? (LlmSchemaComposer.separate(props.model)({
            predicate: props.config.separate as any,
            parameters:
              llmParameters.value satisfies ILlmSchema.ModelParameters[Model] as any,
            equals: props.config.equals ?? false,
          }) as ILlmFunction.ISeparated<Model>)

src/composers/HttpLlmApplicationComposer.ts:237

  • The PR description states this is a "targeted update" that "replaces the use of LlmSchemaComposer.separateParameters with LlmSchemaComposer.separate". However, the actual changes are much more extensive:
  1. The entire LlmSchemaComposer.ts file was completely rewritten (from ~97 lines to 789 lines)
  2. The API changed from model-based curried functions (e.g., method(model)({props})) to direct prop-based functions (e.g., method({props}))
  3. This change affects not just separate/separateParameters, but also parameters, schema, invert, and other methods
  4. The update in this file is incomplete - it only partially updates separateParameters to separate but doesn't fix the API usage pattern, and leaves parameters and schema calls unchanged despite them also having new APIs

The PR description significantly understates the scope of changes.

        ? (LlmSchemaComposer.separate(props.model)({
            predicate: props.config.separate as any,
            parameters:
              llmParameters.value satisfies ILlmSchema.ModelParameters[Model] as any,
            equals: props.config.equals ?? false,
          }) as ILlmFunction.ISeparated<Model>)

src/composers/LlmSchemaComposer.ts:600

  • Variable 'llm' cannot be of type null, but it is compared to an expression of type null.
      llm !== null

src/composers/LlmSchemaComposer.ts:606

  • Variable 'human' cannot be of type null, but it is compared to an expression of type null.
      human !== null

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@samchon samchon merged commit 8bdffda into v6.0 Dec 19, 2025
3 of 4 checks passed
@samchon samchon deleted the feat/llm-schema-composer branch December 19, 2025 03:00
samchon added a commit that referenced this pull request Dec 23, 2025
* Unified to `ILlmSchema`, no more separation. (#213)

* Unified to `ILlmSchema`

* Complete the new `ILlmSchema` type

* detailed description comments

* Update src/structures/ILlmSchema.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/structures/ILlmSchema.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/structures/ILlmSchema.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* prettier

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Unify `ILlmApplication` and `ILlmFunction` too. (#214)

* Universal `LlmSchemaComposer` (#215)

* Universal LlmSchemaComposer

* complete `LlmSchemaComposer.schema()` function

* fix logics

* Universal `LlmTypeChecker` (#216)

* Universal `LlmTypeChecker`

* Fix `LlmSchemaComposer` to utilize `LlmTypeCheckeer`

* JSDoc comments on universal LLM types. (#217)

* Universal `HttpLlm` (#218)

* Universal `HttpLlm`

* Update src/HttpLlm.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/HttpLlm.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* fix configuration comments

* fix more thing

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Remove individual LLM schemas (#219)

* Remove individual LLM schemas

* fix

* Unify test functions about LLM schemas (#220)

* Unify test functions about LLM schemas

* Update test/src/utils/LlmApplicationFactory.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* fixed most of test functions

* fixed most of test functions

* completed everything

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Publish v6

* Re-write README for universal LLM schemas (#221)

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants