From 4d4dab80bb0e872f0b228f7ff4f08aa702e39b8f Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Sat, 4 Jul 2026 21:08:43 -0400 Subject: [PATCH] feat(model-profiles): wrap multi-provider `build_summary` output in `
` toggles (#38664) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `build_summary` in `langchain_model_profiles._summary` now wraps each provider section in a `
` toggle when more than one provider has changes, making multi-provider refresh PR summaries skimmable. Single-provider summaries are unchanged. --- When the `refresh_model_profiles` workflow refreshes data for multiple partner integrations at once, the resulting PR summary can be long and hard to skim โ€” every provider's added/removed/changed rows are rendered flat, one after another. This wraps each provider section in a `
`/`` toggle when more than one provider has changes, so reviewers can expand only the providers they care about. Single-provider summaries stay flat since there's nothing to collapse. The per-provider `### {provider}` heading is stripped inside toggles so the `` tag is the sole label โ€” no duplicated provider name. Also includes two smaller changes that were staged alongside: - The `refresh_model_profiles` and `_refresh_model_profiles` workflow PR-body templates now link to the workflow file instead of bare-text referencing it. - `AGENTS.md` and `CLAUDE.md` document the LangSmith integration test tracing setup: the env vars CI sets, the pytest plugin that bridges `LANGSMITH_TAGS`/`LANGSMITH_METADATA` into `tracing_context`, and the unit-test isolation approach. --------- Signed-off-by: Mason Daugherty Co-authored-by: open-swe[bot] <215916821+open-swe[bot]@users.noreply.github.com> --- .github/workflows/_refresh_model_profiles.yml | 2 +- .github/workflows/refresh_model_profiles.yml | 5 +- AGENTS.md | 16 +++++ CLAUDE.md | 16 +++++ .../langchain_model_profiles/_summary.py | 26 +++++-- .../tests/unit_tests/test_summary.py | 68 ++++++++++++++++++- 6 files changed, 124 insertions(+), 9 deletions(-) diff --git a/.github/workflows/_refresh_model_profiles.yml b/.github/workflows/_refresh_model_profiles.yml index aeab17f5dc8..95b7d1b8a8f 100644 --- a/.github/workflows/_refresh_model_profiles.yml +++ b/.github/workflows/_refresh_model_profiles.yml @@ -69,7 +69,7 @@ on: default: | Automated refresh of model profile data via `langchain-profiles refresh`. - ๐Ÿค– Generated by the `refresh_model_profiles` workflow. + ๐Ÿค– Generated by the [`refresh_model_profiles` workflow](https://github.com/langchain-ai/langchain/blob/master/.github/workflows/refresh_model_profiles.yml). pr-labels: description: "Comma-separated labels to apply to the PR." required: false diff --git a/.github/workflows/refresh_model_profiles.yml b/.github/workflows/refresh_model_profiles.yml index 3824d5e435b..fcf9155b3db 100644 --- a/.github/workflows/refresh_model_profiles.yml +++ b/.github/workflows/refresh_model_profiles.yml @@ -37,10 +37,9 @@ jobs: cli-path: libs/model-profiles add-paths: libs/partners/**/data/_profiles.py pr-body: | - Automated refresh of model profile data for all in-monorepo partner - integrations via `langchain-profiles refresh`. + Automated refresh of model profile data for all in-monorepo partner integrations via `langchain-profiles refresh`. - ๐Ÿค– Generated by the `refresh_model_profiles` workflow. + ๐Ÿค– Generated by the [`refresh_model_profiles` workflow](https://github.com/langchain-ai/langchain/blob/master/.github/workflows/refresh_model_profiles.yml). secrets: MODEL_PROFILE_BOT_CLIENT_ID: ${{ secrets.MODEL_PROFILE_BOT_CLIENT_ID }} MODEL_PROFILE_BOT_PRIVATE_KEY: ${{ secrets.MODEL_PROFILE_BOT_PRIVATE_KEY }} diff --git a/AGENTS.md b/AGENTS.md index 0761b51d677..f5b34910a9a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -295,6 +295,22 @@ Releases are triggered manually via `.github/workflows/_release.yml` with `worki - `.github/workflows/auto-label-by-package.yml` โ€“ Issue labeling by package - `.github/workflows/tag-external-issues.yml` โ€“ Issue external/internal classification +### Integration test tracing (LangSmith) + +Scheduled and manually dispatched integration tests (`integration_tests.yml`) trace every run to LangSmith so failures link back to the originating Actions run. (`_release.yml` runs integration tests too, but does not currently configure LangSmith tracing.) + +**Env vars set by CI:** + +- `LANGSMITH_API_KEY` โ€” authenticates to LangSmith (repo secret, scoped to the "Scheduled testing" GitHub environment in `integration_tests.yml`). +- `LANGSMITH_TRACING: "true"` โ€” enables tracing for the test process. +- `LANGSMITH_PROJECT` โ€” the project traces are sent to. Defaults to `scheduled-testing-py` via a repo variable override: `${{ vars.LANGSMITH_PROJECT || 'scheduled-testing-py' }}`. To change the project, set the `LANGSMITH_PROJECT` repository variable in GitHub settings โ€” do not hardcode it in the workflow. +- `LANGSMITH_TAGS` โ€” comma-separated tags identifying the run: `github-actions`, the matrix working directory (e.g. `libs/partners/openai`), the Python version, and the commit SHA. +- `LANGSMITH_METADATA` โ€” a JSON object built by the "Build LangSmith Metadata" step, containing `github_sha`, `github_run_id`, `github_run_attempt`, `github_run_url`, `github_workflow`, `github_event`, `github_ref`, `working_directory`, and `python_version`. + +**The tracing bridge plugin:** The LangSmith SDK does not natively read `LANGSMITH_TAGS` or `LANGSMITH_METADATA` from the environment. The pytest plugin at `libs/standard-tests/langchain_tests/_langsmith_plugin.py` bridges that gap by entering `langsmith.run_helpers.tracing_context` for the duration of the test session. It only activates when `GITHUB_ACTIONS=true`, so local development is unaffected. Auto-discovered via the `pytest11` entry point in any package that depends on `langchain-tests`. + +**Unit test isolation:** Unit tests must never make network calls or send traces. The `make test` target in the `libs/core` Makefile uses `env -u` to unset the tracing vars (`LANGCHAIN_TRACING_V2`, `LANGCHAIN_API_KEY`, `LANGSMITH_API_KEY`, `LANGSMITH_TRACING`, `LANGCHAIN_PROJECT`) before running pytest. Additionally, `libs/core/tests/unit_tests/runnables/conftest.py` has a session-scoped autouse fixture that explicitly disables tracing for runnable unit tests, restoring the original environment afterward. + ### Adding a new partner to CI When adding a new partner package, update these files: diff --git a/CLAUDE.md b/CLAUDE.md index 0761b51d677..f5b34910a9a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -295,6 +295,22 @@ Releases are triggered manually via `.github/workflows/_release.yml` with `worki - `.github/workflows/auto-label-by-package.yml` โ€“ Issue labeling by package - `.github/workflows/tag-external-issues.yml` โ€“ Issue external/internal classification +### Integration test tracing (LangSmith) + +Scheduled and manually dispatched integration tests (`integration_tests.yml`) trace every run to LangSmith so failures link back to the originating Actions run. (`_release.yml` runs integration tests too, but does not currently configure LangSmith tracing.) + +**Env vars set by CI:** + +- `LANGSMITH_API_KEY` โ€” authenticates to LangSmith (repo secret, scoped to the "Scheduled testing" GitHub environment in `integration_tests.yml`). +- `LANGSMITH_TRACING: "true"` โ€” enables tracing for the test process. +- `LANGSMITH_PROJECT` โ€” the project traces are sent to. Defaults to `scheduled-testing-py` via a repo variable override: `${{ vars.LANGSMITH_PROJECT || 'scheduled-testing-py' }}`. To change the project, set the `LANGSMITH_PROJECT` repository variable in GitHub settings โ€” do not hardcode it in the workflow. +- `LANGSMITH_TAGS` โ€” comma-separated tags identifying the run: `github-actions`, the matrix working directory (e.g. `libs/partners/openai`), the Python version, and the commit SHA. +- `LANGSMITH_METADATA` โ€” a JSON object built by the "Build LangSmith Metadata" step, containing `github_sha`, `github_run_id`, `github_run_attempt`, `github_run_url`, `github_workflow`, `github_event`, `github_ref`, `working_directory`, and `python_version`. + +**The tracing bridge plugin:** The LangSmith SDK does not natively read `LANGSMITH_TAGS` or `LANGSMITH_METADATA` from the environment. The pytest plugin at `libs/standard-tests/langchain_tests/_langsmith_plugin.py` bridges that gap by entering `langsmith.run_helpers.tracing_context` for the duration of the test session. It only activates when `GITHUB_ACTIONS=true`, so local development is unaffected. Auto-discovered via the `pytest11` entry point in any package that depends on `langchain-tests`. + +**Unit test isolation:** Unit tests must never make network calls or send traces. The `make test` target in the `libs/core` Makefile uses `env -u` to unset the tracing vars (`LANGCHAIN_TRACING_V2`, `LANGCHAIN_API_KEY`, `LANGSMITH_API_KEY`, `LANGSMITH_TRACING`, `LANGCHAIN_PROJECT`) before running pytest. Additionally, `libs/core/tests/unit_tests/runnables/conftest.py` has a session-scoped autouse fixture that explicitly disables tracing for runnable unit tests, restoring the original environment afterward. + ### Adding a new partner to CI When adding a new partner package, update these files: diff --git a/libs/model-profiles/langchain_model_profiles/_summary.py b/libs/model-profiles/langchain_model_profiles/_summary.py index 391a1f384ed..d7887a2e03a 100644 --- a/libs/model-profiles/langchain_model_profiles/_summary.py +++ b/libs/model-profiles/langchain_model_profiles/_summary.py @@ -314,18 +314,21 @@ def render_provider_section(provider: str, diff: ProfileDiff) -> str | None: def build_summary(provider_diffs: dict[str, ProfileDiff]) -> str: """Assemble the full Markdown summary across all providers. + When more than one provider has changes, each provider's section is wrapped + in a `
` toggle so the PR body stays skimmable. + Args: provider_diffs: Mapping of provider name to its `ProfileDiff`. Returns: Markdown summary. When nothing changed, a short note is returned. """ - sections = [ - section + provider_sections = [ + (provider, section) for provider in sorted(provider_diffs) if (section := render_provider_section(provider, provider_diffs[provider])) ] - if not sections: + if not provider_sections: return "No model profile data changed." total_added = sum(len(d.added) for d in provider_diffs.values()) @@ -333,9 +336,24 @@ def build_summary(provider_diffs: dict[str, ProfileDiff]) -> str: total_changed = sum(len(d.changed) for d in provider_diffs.values()) headline = ( f"**{total_added} added ยท {total_removed} removed ยท " - f"{total_changed} changed** across {len(sections)} provider(s)." + f"{total_changed} changed** across {len(provider_sections)} provider(s)." ) + # Wrap each section in a
toggle only when multiple providers + # changed, so a single-provider summary stays flat. + wrap = len(provider_sections) > 1 + sections = [] + for provider, section in provider_sections: + if not wrap: + sections.append(section) + continue + # Strip the "### {provider}" heading so the tag is the sole + # label for the toggle. + body = section.removeprefix(f"### {provider}").lstrip("\n") + sections.append( + f"
\n{provider}\n\n{body}\n\n
" + ) + return "\n\n".join(["## Summary of changes", headline, *sections]) diff --git a/libs/model-profiles/tests/unit_tests/test_summary.py b/libs/model-profiles/tests/unit_tests/test_summary.py index f0615aad28b..5bb0946f5bf 100644 --- a/libs/model-profiles/tests/unit_tests/test_summary.py +++ b/libs/model-profiles/tests/unit_tests/test_summary.py @@ -166,6 +166,7 @@ def test_build_summary_headline() -> None: assert "1 added" in summary assert "1 removed" in summary assert "1 changed" in summary + assert "across 1 provider(s)." in summary def test_build_summary_no_changes() -> None: @@ -308,7 +309,72 @@ def test_build_summary_multi_provider_sorted() -> None: diff_a = diff_profiles({}, {"a": {"name": "A"}}) diff_z = diff_profiles({}, {"z": {"name": "Z"}}) summary = build_summary({"zzz": diff_z, "aaa": diff_a}) - assert summary.index("### aaa") < summary.index("### zzz") + assert summary.index("aaa") < summary.index( + "zzz" + ) + + +def test_build_summary_multi_provider_wraps_in_toggles() -> None: + """More than one changed provider gets each wrapped in a
toggle.""" + diff_a = diff_profiles({}, {"a": {"name": "A"}}) + diff_z = diff_profiles({}, {"z": {"name": "Z"}}) + summary = build_summary({"aaa": diff_a, "zzz": diff_z}) + assert summary.count("
") == 2 + assert summary.count("
") == 2 + assert "aaa" in summary + assert "zzz" in summary + # The "### provider" headings are stripped inside toggles. + assert "### aaa" not in summary + assert "### zzz" not in summary + # Headline counts only the wrapped providers. + assert "across 2 provider(s)." in summary + # Each toggle keeps its section body (guards against over-stripping): the + # per-provider "N added" marker and model rows survive, and each row lands + # after its own label rather than being dropped or misattributed. + assert summary.count("1 added") == 2 + assert ( + summary.index("aaa") + < summary.index("- `a`") + < summary.index("zzz") + < summary.index("- `z`") + ) + + +def test_build_summary_multi_provider_preserves_removed_and_changed() -> None: + """Multi-provider toggles keep removed and changed bodies, not just added.""" + diff = diff_profiles(extract_profiles(_OLD_SOURCE), extract_profiles(_NEW_SOURCE)) + other = diff_profiles({}, {"m": {"name": "M"}}) + summary = build_summary({"openai": diff, "zzz": other}) + assert summary.count("
") == 2 + # The realistic diff's removed and changed phrases survive the heading strip. + assert "1 removed" in summary + assert "`old-model`" in summary + assert "max output tokens 4,096 โ†’ 16,384" in summary + # Changed content stays inside the openai toggle, before the next provider. + assert ( + summary.index("openai") + < summary.index("max output tokens 4,096 โ†’ 16,384") + < summary.index("zzz") + ) + + +def test_build_summary_single_provider_no_toggle() -> None: + """A single changed provider renders as a plain section without toggles.""" + diff = diff_profiles(extract_profiles(_OLD_SOURCE), extract_profiles(_NEW_SOURCE)) + summary = build_summary({"openai": diff}) + assert "
" not in summary + assert "### openai" in summary + assert "across 1 provider(s)." in summary + + +def test_build_summary_empty_diff_filtered_from_count() -> None: + """Providers with empty diffs are excluded from the toggle/count decision.""" + diff_a = diff_profiles({}, {"a": {"name": "A"}}) + summary = build_summary({"aaa": diff_a, "empty": ProfileDiff()}) + # Only one provider actually changed, so no toggles and the count is 1. + assert "
" not in summary + assert "### aaa" in summary + assert "across 1 provider(s)." in summary def test_summarize_removed_when_file_deleted(tmp_path: Path) -> None: