Skip to content

feat(api): update API spec from langfuse/langfuse 9a29212 - #1918

Closed
langfuse-bot wants to merge 1 commit into
mainfrom
api-spec-bot-9a29212-36876693802-1
Closed

langfuse-bot wants to merge 1 commit into
mainfrom
api-spec-bot-9a29212-36876693802-1

Conversation

@langfuse-bot

@langfuse-bot langfuse-bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

RetriggerConfidence Score: 4/5

The skills API needs correction before merge because its new accessors violate an explicit repository import requirement; the example and URL-handling issues should also be addressed.

Summary

The PR regenerates the API client with a new unstable skills API, additional query parameters and response fields, pagination-model reuse, and updated ingestion guidance.

  • The new skills API has an unusable import in its creation examples and does not safely handle URL delimiters in skill names.
  • The skills accessors violate the repository’s module-level import requirement.

Reviews (1) · Last reviewed commit: "feat(api): update API spec from langfuse..."

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

@claude review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

No review was started: this request came from a bot account. Manual reviews can only be requested by someone with write access to this repository. Ask a maintainer to comment @claude review, or have your automation post the comment from a user account with write access.

Tip: disable this comment in your organization's Code Review settings.

HttpResponse[SkillVersion]
"""
_response = self._client_wrapper.httpx_client.request(
f"api/public/unstable/skills/{jsonable_encoder(skill_name)}",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Skill names alter request URLs If a skill name contains a URL delimiter such as ? or #, this interpolation places it directly in the request path: jsonable_encoder leaves strings unchanged, and the transport does not encode path segments. The request then targets a different URL instead of the named skill. The same pattern affects the update, label, and delete routes.

Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/api/unstable/skills/raw_client.py
Line: 315

Comment:
**Skill names alter request URLs** If a skill name contains a URL delimiter such as `?` or `#`, this interpolation places it directly in the request path: `jsonable_encoder` leaves strings unchanged, and the transport does not encode path segments. The request then targets a different URL instead of the named skill. The same pattern affects the update, label, and delete routes.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟡 langfuse/api/evaluation_commons/types/evaluation_rule_filter.py — SDK users cannot create or update evaluation rules using the new is set/is not set stringObject operators that this same PR's docs now advertise, because the backing enum was never regenerated to add them. EvaluationRuleFilter_StringObject.operator (line 456) and the docstring at line 416 both reference EvaluationRuleStringFilterOperator, which still only has EQUALS/CONTAINS/DOES_NOT_CONTAIN/STARTS_WITH/ENDS_WITH (langfuse/api/evaluation_commons/types/evaluation_rule_string_filter_operator.py, untouched by this diff). Constructing EvaluationRuleFilter_StringObject(operator="is set", ...) raises a pydantic ValidationError since "is set" isn't a member. Fix: regenerate/add IS_SET and IS_NOT_SET members to EvaluationRuleStringFilterOperator so the create/update evaluation-rule request types can actually send the documented operators.

    Why this was flagged

    Any caller using the typed Python client to build an EvaluationRuleFilter_StringObject (used by create_evaluation_rule_request.py/update_evaluation_rule_request.py via the EvaluationRuleFilter union) with operator 'is set' or 'is not set' hits this. The field operator at evaluation_rule_filter.py:456 is typed as EvaluationRuleStringFilterOperator, a strict Python Enum with only 5 members (evaluation_rule_string_filter_operator.py, not in this diff's changed-file list). Pydantic validates Enum fields by exact membership, so passing the new value raises ValidationError instead of constructing the filter. The base branch didn't document these operators at all, so no regression for existing values, but the new capability described at evaluation_rule_filter.py:416 and in many other docstrings this diff touches is unusable through the generated SDK types; users must drop to raw dict/JSON to bypass validation.

    Verification: Severity: nit (the functional inability is pre-existing; the diff only introduces a docs/enum mismatch). The docstrings were changed (evaluation_rule_filter.py line 416) to advertise "is set", "is not set". EvaluationRuleFilter_StringObject.operator at line 456 is typed EvaluationRuleStringFilterOperator, whose enum (evaluation_commons/types/evaluation_rule_string_filter_operator.py, lines 11-15) is unchanged, still only EQUALS/CONTAINS/DOES_NOT_CONTAIN/STARTS_WITH/ENDS_WITH.

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