Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions tests/e2e/test_core_sdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,13 +360,6 @@ def test_create_text_score():
)


@pytest.mark.xfail(
strict=False,
reason=(
"Needs the Langfuse platform release that adds timestamp to POST /scores "
"(langfuse prep-js-v6-py-v5-sdk-majors). Remove this marker once it ships."
),
)
def test_create_score_with_custom_timestamp():
langfuse = Langfuse()

Expand Down
6 changes: 3 additions & 3 deletions tests/e2e/test_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,7 @@ def main():
in (trace.output or {}).get("context", {}).get("nested", "")
and any(
"@@@langfuseMedia:type=application/pdf|id="
in observation.metadata.get("context", {}).get("nested", "")
in observation.metadata.get("context.nested", "")
for observation in trace.observations
if observation.metadata
)
Expand All @@ -1133,10 +1133,10 @@ def main():
observation = trace_data.observations[0]
assert (
"@@@langfuseMedia:type=application/pdf|id="
in observation.metadata["context"]["nested"]
in observation.metadata["context.nested"]
)
parsed_reference_string = LangfuseMedia.parse_reference_string(
observation.metadata["context"]["nested"]
observation.metadata["context.nested"]
)
assert parsed_reference_string["content_type"] == "application/pdf"
assert parsed_reference_string["media_id"] is not None
Expand Down
12 changes: 6 additions & 6 deletions tests/e2e/test_media.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ def test_replace_media_reference_string_in_object():
is_result_ready=lambda observations: (
re.match(
r"^@@@langfuseMedia:type=audio/wav\|id=.+\|source=base64_data_uri@@@$",
observations[0].metadata.get("context", {}).get("nested", ""),
observations[0].metadata.get("context.nested", ""),
)
is not None
),
)
assert len(fetched_observations) == 1
fetched_observation = fetched_observations[0]
media_ref = fetched_observation.metadata["context"]["nested"]
media_ref = fetched_observation.metadata["context.nested"]
assert re.match(
r"^@@@langfuseMedia:type=audio/wav\|id=.+\|source=base64_data_uri@@@$",
media_ref,
Expand All @@ -53,20 +53,20 @@ def test_replace_media_reference_string_in_object():
)

expected_base64 = f"data:audio/wav;base64,{base64_audio}"
assert resolved_obs["metadata"]["context"]["nested"] == expected_base64
assert resolved_obs["metadata"]["context.nested"] == expected_base64

span2 = langfuse.start_observation(
name=f"2-{mock_trace_name}",
metadata={"context": {"nested": resolved_obs["metadata"]["context"]["nested"]}},
metadata={"context": {"nested": resolved_obs["metadata"]["context.nested"]}},
).end()

langfuse.flush()

fetched_observations2 = wait_for_observations(
span2.trace_id,
is_result_ready=lambda observations: (
observations[0].metadata.get("context", {}).get("nested") == media_ref
observations[0].metadata.get("context.nested") == media_ref
),
)
assert len(fetched_observations2) == 1
assert fetched_observations2[0].metadata["context"]["nested"] == media_ref
assert fetched_observations2[0].metadata["context.nested"] == media_ref
10 changes: 3 additions & 7 deletions tests/live_provider/test_openai.py
Original file line number Diff line number Diff line change
Expand Up @@ -999,17 +999,13 @@ def test_structured_output_response_format_kwarg(openai):

langfuse.flush()

generation = wait_for_observations(
name=generation_name, type="GENERATION", expand_metadata="response_format"
)
generation = wait_for_observations(name=generation_name, type="GENERATION")

assert len(generation) != 0
assert generation[0].name == generation_name
assert generation[0].metadata["someKey"] == "someResponse"
assert generation[0].metadata["response_format"] == {
"type": "json_schema",
"json_schema": json_schema,
}
assert generation[0].metadata["response_format.type"] == "json_schema"
assert generation[0].metadata["response_format.json_schema.name"] == "math_response"
Comment on lines +1007 to +1008

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve the full response-format assertion

Assert the flattened equivalent of the complete response_format, not only its type and name. If the OpenAI instrumentation drops or corrupts strict, the schema properties, required fields, or additionalProperties, this test now passes despite previously verifying the entire object; checking all expected flattened leaf paths and serialized values would retain that regression coverage.

AGENTS.md reference: AGENTS.md:L32-L34

Useful? React with 👍 / 👎.

Comment on lines +1007 to +1008

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 nit (optional): Maintainers lose coverage of the recorded json_schema body. The new assertions would still pass if the SDK dropped or mangled schema in response_format metadata. Only response_format.type and response_format.json_schema.name are checked now, though the flattened schema still has string leaves that can be asserted. Fix: also assert a few string-valued schema paths, e.g. metadata["response_format.json_schema.schema.type"] == "object" and metadata["response_format.json_schema.schema.properties.final_answer.type"] == "string".

Why this was flagged

On the base branch, tests/live_provider/test_openai.py compared generation[0].metadata["response_format"] with the full {"type": "json_schema", "json_schema": json_schema}. So a regression in _get_structured_output_metadata / _serialize_openai_value (langfuse/openai.py:250-274) that lost or changed the nested schema would fail the test. After the change, lines 1007-1008 check only two top-level string leaves. Such a regression now passes silently. The PR says arrays and booleans are JSON-stringified at their path, which explains skipping strict and required. It does not explain skipping string leaves like schema.type or schema.properties.final_answer.type, which come back as plain strings at their dotted paths.

Verification: The base test asserted generation[0].metadata["response_format"] == {"type": "json_schema", "json_schema": json_schema}, covering the full schema (test_openai.py lines 961-984). Lines 1007-1008 now assert only response_format.type and response_format.json_schema.name, so a regression that lost the schema would pass silently.

Comment on lines +1007 to +1008

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 Actual schema goes unchecked

Replacing the full response_format comparison with only type and name lets this test pass if the captured metadata drops json_schema.schema entirely. Keep assertions for short string-valued leaves such as response_format.json_schema.schema.type and response_format.json_schema.schema.properties.final_answer.type. These do not need array or boolean decoding.

Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/live_provider/test_openai.py
Line: 1007-1008

Comment:
**Actual schema goes unchecked**

Replacing the full `response_format` comparison with only `type` and `name` lets this test pass if the captured metadata drops `json_schema.schema` entirely. Keep assertions for short string-valued leaves such as `response_format.json_schema.schema.type` and `response_format.json_schema.schema.properties.final_answer.type`. These do not need array or boolean decoding.

---

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


assert generation[0].input == [
{"role": "system", "content": "You are a helpful math tutor."},
Expand Down
Loading