✨ feat: Add the Classification Port - #561
danny-avila wants to merge 5 commits into
Conversation
A typed question in, a calibrated answer out: `src/classification/` carries the port that
LibreChat PR #16180 introduced under `packages/api` and that codegraph mirrors in ESM, so the
product, the graph and any other consumer share one implementation of the contract a System One
host (TypeSafe's Jev, directly or through a gateway) answers.
- types: boolean / choice / score questions, answers with a probability or a calibrated
confidence and distribution, `Classifier`, `ClassificationError` with typed failures,
`ClassificationDialect`, `ClassificationProviderSettings`
- dialect: boolean ↔ `noul`; a string yes-criterion becomes the `{true}` pair a System One host wants
- transport: one deadline for the whole call, bounded retries on 429/5xx/network honouring
retry-after, an `onAnswered` hook instead of a logger dependency
- http: the host over HTTP, with request/response wrapping for hosts that nest the envelope
- presets: typesafe, openrouter, cloudflare, http; `createClassifier(settings, apiKey)`
- questions: `booleanQuestion`, `choiceQuestion`, `scoreQuestion`
- seven jest tests with a fake fetch; `tsc --noEmit` clean
No LibreChat type is imported: the SDK holds the port, consumers hold their configuration.
…assifier A host whose bearer expires (the ClickHouse inference gateway mints an hourly Okta token) can be given a function instead of a key. The transport calls it before each request and once more with refresh: true after a 401, then retries that request; a 403 is a scope refusal and is not retried. The `clickhouse` preset points at the gateway's System One route.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e695493310
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return Object.hasOwn(CLASSIFICATION_PRESETS, name) | ||
| ? CLASSIFICATION_PRESETS[name] | ||
| : null; |
There was a problem hiding this comment.
This returns the shared object stored in CLASSIFICATION_PRESETS, so a caller that customizes it in place (for example, setting the typesafe preset's requiresAuth to false or changing its baseURL) silently changes every subsequent lookup. In a long-lived or multi-tenant process, later classifiers can consequently skip mandatory authentication or send bearer credentials to the mutated endpoint; return a defensive copy and prevent direct mutation of the exported preset map.
AGENTS.md reference: AGENTS.md:L26-L28
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d46ae34835e4c73d05aafe85fdc93c14a4088938. The exported registry and each preset are frozen, and classificationPreset() returns a caller-owned copy. A regression mutates one tenant's copy and verifies that later lookups retain the canonical endpoint and required authentication.
| throw interrupted(); | ||
| } | ||
| try { | ||
| const key = await waitFor(resolveKey(refreshKey, signal)); |
There was a problem hiding this comment.
When apiKey is a credential function, this calls the minter on every 429, 5xx, or network retry even though the public type describes it as a per-request minter. Besides repeatedly invoking an expensive or rate-limited minting service, a refreshed token is also lost if its first request gets a retryable failure: refreshKey is reset and the next attempt asks for the old, non-refreshed credential again, which can turn a recoverable 500-after-401 sequence into a final 401. Resolve once per classification call and replace the cached token only for the one explicit 401 refresh.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d46ae34835e4c73d05aafe85fdc93c14a4088938. The bearer token is cached per classification call, invalidated only by the first 401, and the refreshed token survives later 5xx and 429 retries. Tests cover a 401/503/429/200 sequence, a plain 429 retry, and independent concurrent calls.
| input = JSON.stringify({ | ||
| state: request.state, | ||
| questions: request.questions, | ||
| }); |
There was a problem hiding this comment.
When a traced classification state contains a tool message or server-tool result and tool-output redaction is enabled, serializing the whole state into the text of a HumanMessage hides the tool structure from redactValue in src/langfuseToolOutputTracing.ts, which leaves nested strings unchanged. The resulting generation input therefore exports the original tool payload even though the run requested redaction; redact before stringification or retain a structured representation that the span processor can recognize.
AGENTS.md reference: AGENTS.md:L147-L147
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d46ae34835e4c73d05aafe85fdc93c14a4088938. Structured-chat prompts carry a narrow marker; the Langfuse span processor now parses the marked state, applies its existing tool and server-tool redaction rules before export, and fails closed for malformed or truncated marked prompts. The provider still receives the original classification input. A focused tracing regression covers selective and global redaction and untouched public tool output.
There was a problem hiding this comment.
Further self-review in f88615413542161cd8db44dd7bf406bff3f47b0d corrected the earlier selective-redaction claim: a free-form classifier state or question can quote private tool output without retaining the tool name. I reproduced that leak. Under any active tool-output redaction policy the trace processor now drops the entire marked classifier prompt, including otherwise-public state; the provider request is unchanged. The previously posted claim that public state remains visible under selective redaction is superseded. New regression covers the unstructured state and question path.
| for (const [key, value] of entries) { | ||
| if (!isProbability(value)) { | ||
| return undefined; |
There was a problem hiding this comment.
The response boundary validates each probability independently but never checks the aggregate, so malformed replies such as { a: 0.8, b: 0.8 } or an all-zero score distribution are returned as measured distributions. Callers can then make routing or threshold decisions from values that cannot represent a probability distribution; validate that a nonempty choice or score distribution sums to approximately one before accepting it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d46ae34835e4c73d05aafe85fdc93c14a4088938. Measured choice and score maps now cover every requested option and sum to one within a bounded rounding tolerance. Regressions reject duplicate 0.8 values, incomplete maps, zero-only distributions, and oversized rounding allowances, while accepting rounded valid distributions.
|
Self-review handoff for PR #561 at exact pushed head This head resolves all four inline findings on the earlier head: immutable presets across tenants, cached and refreshed per-call credentials across retries, nested classifier-prompt tool-output redaction before Langfuse export, and complete normalized measured distributions. It also moves structured-chat validation inside the abortable deadline. Tests cover each previously failing case. Local checks passed: 27 classification tests, 204 tracing tests, full workspace TypeScript typecheck, touched-file lint/import order/formatting, circular-dependency check, and package build. CI for this exact head failed before creating jobs because the current main reusable workflow has duplicate YAML keys. No CI checks ran on this head. A maintainer can trigger a new Codex review for this SHA if desired; a Lia GitHub App comment cannot trigger one. |
|
Review handoff for draft agents PR #561 at exact remote head Further invariant review found and fixed two issues missed at the preceding head: private tool results copied into free-form classifier state or question text escaped selective Langfuse redaction, and a monotonic timeout could settle before its timer callback fired without aborting the fetch signal. Marked classifier prompts now fail closed in traces under any active tool-output redaction policy. Deadline checks now abort the in-flight signal and observe late rejected tasks, including when synchronous preparation crosses the deadline. No request content or provider behavior is changed by trace redaction. Local verification on this head: 234 passed tests across 13 focused classification and Langfuse suites, workspace TypeScript typecheck, touched-file lint/import order/formatting, circular-dependency check, ESM/CJS exports, package build, and diff checks. CI for this head failed before creating any jobs; the current main workflow has duplicate YAML keys. A maintainer can trigger a new Codex review for this exact SHA. A Lia GitHub App comment does not initiate that review. |
This branch has not been deployed
2 participants
Summary
Keep
Classifieras a small SDK contract rather than a chat-model subclass. Jev and self-hosted Laya use the same System One HTTP adapter; an already configured chat model can be injected through a separate strict structured-output adapter. Laya is a data-only preset with an operator-supplied endpoint, optional bearer authentication, and no forced checkpoint.Semantics and safety
probability: number. A chat-only boolean has{ decision: boolean, probability: null }; choice distributions and token usage arenullwhen unmeasured. Missing HTTP answers are typed asundefined, while malformed or unexpected answers fail explicitly.confidencedefinitions, so thresholds cannot be transferred without evaluating the actual checkpoint and use case.Verification
npx jest src/classification langfuse deterministic-trace-id --runInBand --silent: 234 passed across 13 focused suites (29 classifier, 205 tracing), including real LangChain OpenAI and Anthropic strict-mode paths with mocked HTTP responses.npx tsc --noEmit -p tsconfig.json --pretty false: passed.npx eslintandnpm run sort-imports:checkon classifier and touched tracing files, touched-filenpx prettier --check, andgit diff --check: passed.npm run buildandnpm run check:circular-deps: passed. ESM and CommonJS root exports checked.CI status
CI for this head failed before any jobs started. The current
main/.github/workflows/validate.ymlhas five duplicate YAML map keys, and other PRs have the same zero-job failure. No CI test or lint job ran for this head; this upstream workflow issue is separate from the local verification above.Self-review follow-up
Four inline review findings from
e69549331079ccdd0416bd7060110cc8221ebce4were reproduced and fixed ind46ae34835e4c73d05aafe85fdc93c14a4088938: presets and their registry are immutable across tenants; per-call bearer minting and the one 401 refresh persist over retries; marked structured-chat prompts keep the original provider request but preserve Langfuse tool-output redaction before trace export; measured choice/score distributions are complete and normalized within a bounded tolerance. Independent review also moved strict-chat question preparation inside the request deadline, so pre-aborted calls never inspect dynamic questions. New regressions cover all five issues.Further invariant review found two gaps fixed in
f88615413542161cd8db44dd7bf406bff3f47b0d. A private tool result copied into free-form classifier state or instructions has no tool identity after stringification, so selective field redaction could export it. The trace processor now drops the whole marked classifier prompt under any active tool-output redaction policy (provider requests are unchanged); a regression first reproduced the leak. A monotonic deadline can also expire before the timer callback runs, returning while fetch stays active. The deadline now aborts its controller on expiry, honors expiry during synchronous preparation errors, and observes late promise rejections. Regressions reproduced both paths before the fix.Rollout
Keep this PR in draft until the shared SDK is ready to publish. Migrate the duplicate LibreChat port in #16180, then adjust the probability consumers and fallbacks in #16181 separately. Live Jev/Laya comparisons, calibration, latency and cost evaluation, and LibreChat consumer tests have not been run in this agents PR.