chore: improve agent development setup #1642
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| version = 1 | ||
| name = "langfuse-python" | ||
|
|
||
| [setup] | ||
| script = "bash scripts/codex/setup.sh" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| ## What does this PR do? | ||
|
|
||
| > PR title must follow Conventional Commits, for example `feat: add dataset scoring helper` or `fix(openai): preserve trace context`. | ||
|
|
||
| Fixes # | ||
|
|
||
| ## Type of change | ||
|
|
||
| - [ ] Bug fix | ||
| - [ ] New feature | ||
| - [ ] Breaking change | ||
| - [ ] Refactor | ||
| - [ ] Documentation update | ||
| - [ ] Tooling, CI, or repo maintenance | ||
|
|
||
| ## Verification | ||
|
|
||
| List the main commands you ran: | ||
|
|
||
| ```bash | ||
|
|
||
| ``` | ||
|
|
||
| ## Checklist | ||
|
|
||
| - [ ] I self-reviewed the diff using `code_review.md`. | ||
| - [ ] I added or updated tests for behavior changes. | ||
| - [ ] I updated docs, examples, or `.env.template` if needed. | ||
| - [ ] I did not hand-edit generated files; if generated files changed, I used the upstream regeneration path. | ||
| - [ ] I did not commit secrets or credentials. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| --- | ||
| name: "Validate PR Title" | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: | ||
| - "**" | ||
| types: | ||
| - opened | ||
| - edited | ||
| - synchronize | ||
| - reopened | ||
|
|
||
| permissions: {} | ||
|
|
||
| jobs: | ||
| validate-pr-title: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| statuses: write | ||
| pull-requests: read | ||
| steps: | ||
| - name: Validate PR title follows conventional commits | ||
| uses: amannn/action-semantic-pull-request@48f256284bd46cdaab1048c3721360e808335d50 # v6.1.1 | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| with: | ||
| types: | | ||
| feat | ||
| fix | ||
| docs | ||
| style | ||
| refactor | ||
| perf | ||
| test | ||
| build | ||
| ci | ||
| chore | ||
| revert | ||
| security | ||
| requireScope: false | ||
| validateSingleCommit: false | ||
| ignoreLabels: | | ||
| bot | ||
| ignore-semantic-pull-request | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Langfuse Python SDK Review Checklist | ||
|
|
||
| Use this checklist for `/review`, PR review, or self-review before handoff. | ||
|
|
||
| ## Priorities | ||
|
|
||
| - Findings first: correctness bugs, regressions, security/privacy risks, performance issues with real impact, and missing tests for risky behavior. | ||
| - Keep line references tight and actionable. | ||
| - If no findings, say so explicitly and mention any residual risk or unrun verification. | ||
|
|
||
| ## SDK Correctness | ||
|
|
||
| - Public SDK behavior should remain backwards compatible unless the PR is explicitly breaking. | ||
| - Prefer `LANGFUSE_BASE_URL`; `LANGFUSE_HOST` is deprecated and should only appear in compatibility paths or tests. | ||
| - Check shutdown, flushing, background task, and resource-manager changes for races, dropped events/scores/media, daemon-thread leaks, and hanging interpreter shutdown. | ||
| - OpenTelemetry changes should preserve context propagation, span parenting, exporter-local testability, and idempotent instrumentation setup. | ||
| - OpenAI and LangChain instrumentation should avoid brittle assertions on provider internals; prefer stable exporter-local behavior in unit tests. | ||
|
|
||
| ## API And Generated Code | ||
|
|
||
| - Do not hand-edit `langfuse/api/`; regenerate it from the upstream Fern/OpenAPI source. | ||
| - Public API or serialization changes should include tests for request shape, response shape, and backwards-compatible aliases when relevant. | ||
| - Update README examples, `.env.template`, or generated reference docs when changed behavior would make them stale. | ||
|
|
||
| ## Tests And CI | ||
|
|
||
| - Unit tests must not require a running Langfuse server. | ||
| - E2E tests should use bounded polling helpers from `tests/support/`, not raw `sleep()`. | ||
| - New e2e files must be named `tests/e2e/test_*.py` so mechanical CI sharding includes them. | ||
| - Use `serial_e2e` only for tests that are unsafe with shared-server concurrency. | ||
| - Live-provider tests should assert stable provider-facing behavior, not exact observation counts unless counts are the behavior under test. | ||
|
|
||
| ## Python Style | ||
|
|
||
| - Exception messages should not inline f-string literals in `raise` statements; build the message in a variable first. | ||
| - Keep edits ASCII-only unless the file already uses Unicode or Unicode is clearly required. | ||
| - Keep changes scoped; avoid opportunistic refactors. | ||
| - Never commit secrets or credentials. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" | ||
| cd "$repo_root" | ||
|
|
||
| uv sync --locked | ||
| uv cache prune --ci >/dev/null 2>&1 || true |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" | ||
| cd "$repo_root" | ||
|
|
||
| uv run --frozen ruff check . | ||
| uv run --frozen mypy langfuse --no-error-summary | ||
| uv run --frozen pytest -n auto --dist worksteal tests/unit |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" | ||
| cd "$repo_root" | ||
|
|
||
| if ! command -v uv >/dev/null 2>&1; then | ||
| python3 -m pip install --user "uv==0.11.2" | ||
|
Contributor
There was a problem hiding this comment.
Prompt To Fix With AIThis is a comment left during a code review.
Path: scripts/codex/setup.sh
Line: 8
Comment:
**Pinned uv version may conflict with lockfile format**
`uv==0.11.2` is installed as the fallback, but `uv sync --locked` is called immediately after with the repo's existing `uv.lock`. If the lockfile was generated with a different uv version (older or newer), the sync can fail due to lockfile format incompatibilities. Consider either pinning to the same version used to generate the lockfile, or adding a comment explaining why `0.11.2` was chosen and how it should be kept in sync with the project's own uv dependency.
How can I resolve this? If you propose a fix, please make it concise. |
||
| export PATH="$HOME/.local/bin:$PATH" | ||
| fi | ||
|
|
||
| uv sync --locked | ||
| uv run --frozen python --version | ||
|
Check warning on line 13 in scripts/codex/setup.sh
|
||
|
Comment on lines
+7
to
+13
There was a problem hiding this comment. 🟡 The fallback bootstrap in Extended reasoning...What the bug is
if ! command -v uv >/dev/null 2>&1; then
python3 -m pip install --user "uv==0.11.2"
export PATH="$HOME/.local/bin:$PATH"
fiTwo things go wrong when
Why existing code does not prevent itThe author clearly anticipated the missing- ImpactThis only triggers when (a) the fallback branch fires ( Step-by-step proofConcrete reproducer on a typical PEP 668 base image (e.g.
Alternative path (non-PEP 668 system, e.g. older base image):
How to fixSwitch the fallback to the canonical uv installer, which sidesteps PEP 668 and writes shell-rc entries so subsequent shells inherit the PATH: if ! command -v uv >/dev/null 2>&1; then
curl -LsSf https://astral.sh/uv/install.sh | sh
export PATH="$HOME/.local/bin:$PATH"
fiAlternatives: |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
pull_requesthere makes this action fail for fork-based pull requests (the action’s own docs state this trigger only works for branches in the same repository). In any workflow that accepts external contributors, this causes the required PR-title check to error on fork PRs and can block merges until a maintainer intervenes. Trigger this workflow withpull_request_targetinstead (with minimal permissions) so title validation runs reliably for forks.Useful? React with 👍 / 👎.