code-review

---
name: code-review
description: >-
  Reviews changes in the langchain-azure monorepo using package-specific
  knowledge of langchain-azure-ai, langchain-azure-compute,
  langchain-azure-cosmosdb, langchain-azure-postgresql, langchain-azure-storage,
  langchain-sqlserver, and langchain-azure-dynamic-sessions, together with the
  LangChain, LangGraph, Deep Agents, and Azure SDK contracts each package must
  satisfy. Use this skill whenever reviewing a pull request or diff, checking
  code for bugs or regressions, or assessing changes under libs/, samples/, or
  .github/ in this repository, including when the request is only to "review",
  "check", "look at", or "give feedback on" a change, and even when no package
  is named explicitly.
license: MIT
---

# Reviewing langchain-azure changes

Every directory under `libs/` is a separately versioned, separately released
package with its own maintainers, dependency manager, conventions, and upstream
contracts. A finding is only useful if it is correct *for the package it lands
in*, so the first job in any review is to work out which package changed and
load that package's rules before judging anything.

## What to report

Report defects the change introduces: incorrect behavior, broken edge cases,
regressions in released public API, violations of an upstream contract
(LangChain, LangGraph, Deep Agents, Azure SDK), breakage on a supported Python
version, and security, credential-leak, data-loss, resource-leak, or
concurrency problems.

Stay silent about everything else. In particular, do not comment on formatting,
naming, or docstring wording that `ruff` and `mypy` already enforce; do not
restate what the diff does; do not raise pre-existing issues the change merely
touches; and do not suggest refactors that are not required for correctness.
Returning no comments on a correct change is a good review — never manufacture
findings to look thorough.

Copilot code review never sees `**/*.lock`, `**/*.svg`, `**/*.log`, or
`**/dist/**`, so `uv.lock` is invisible to you. Excluded files are also stripped
from the file list you receive, so you cannot tell a lockfile that was never
updated from one that was updated and hidden from you. Never write a finding
about lockfile contents *or* lockfile presence; instead, see the lockfile
gotcha below.

## Review workflow

1. **Identify the packages touched.** Group changed files by `libs/<package>/`.
   Treat each package as an independent review.
2. **Load the package's rules** from the routing table below, plus any
   `AGENTS.md` or `.github/copilot-instructions.md` along the changed path. The
   repository root `AGENTS.md` is already in your context; do not re-derive it.
3. **Read enough surrounding code** to know what the changed lines actually do:
   the function, its callers, its sync or async twin, and the nearest tests.
   Never review a hunk in isolation.
4. **Check the upstream contract** for the base class being implemented, using
   [ecosystem contracts](references/ecosystem-contracts.md) and
   [Azure SDK contracts](references/azure-sdk-contracts.md).
5. **Confirm each finding** before writing it. A finding must satisfy all four:
   the changed code causes it; a realistic supported input or code path reaches
   it; the consequence is concrete; and you can point at the specific lines. If
   any of these is missing, drop it.

## Package routing

Read the file for each package that changed. Skip the rest.

| Changed path | Package | Read |
|---|---|---|
| `libs/azure-ai/` | `langchain-azure-ai` | [azure-ai.md](references/azure-ai.md) |
| `libs/azure-compute/` | `langchain-azure-compute` | [azure-compute.md](references/azure-compute.md) |
| `libs/azure-cosmosdb/` | `langchain-azure-cosmosdb` | [azure-cosmosdb.md](references/azure-cosmosdb.md) |
| `libs/azure-postgresql/` | `langchain-azure-postgresql` | [azure-postgresql.md](references/azure-postgresql.md) |
| `libs/azure-storage/` | `langchain-azure-storage` | [azure-storage.md](references/azure-storage.md) |
| `libs/sqlserver/` | `langchain-sqlserver` | [sqlserver.md](references/sqlserver.md) |
| `libs/azure-dynamic-sessions/` | deprecated | [azure-dynamic-sessions.md](references/azure-dynamic-sessions.md) |
| `.github/`, `samples/`, root docs | repo infrastructure | [repo-infrastructure.md](references/repo-infrastructure.md) |

Also read [ecosystem contracts](references/ecosystem-contracts.md) when the
change implements or overrides a LangChain, LangGraph, or Deep Agents base
class, and [Azure SDK contracts](references/azure-sdk-contracts.md) when it
constructs an Azure client, handles credentials, or maps service errors.

## Repository gotchas

These are the mistakes that pass local review and break later. They are
specific to this repository and override any general instinct.

- **Never report a missing or stale `uv.lock`.** CI runs `uv lock --check` on
  every touched package and fails the PR if a lockfile is stale or absent, so
  this is already gated far more reliably than you can infer it. You cannot
  observe lockfiles: they are excluded from your view *and* omitted from the
  file list you receive. A reviewed-file count below the PR's total changed-file
  count (for example "30/37 files reviewed") means excluded files exist, and on
  a dependency change those are almost always the very `uv.lock` updates you
  would otherwise flag as missing. Absence of evidence here is not evidence of
  absence — stay silent and let CI decide.
- **Raising the minimum Python version is not a breaking change here.** The
  repository follows a Python support policy, and dropping an end-of-life
  interpreter changes no API or behavior on any still-supported version.
  `requires-python` makes older runtimes resolve to the previous release rather
  than install an incompatible one, so nothing breaks silently. These ship as
  patch releases; demanding a `**[Breaking change]:**` marker on one
  contradicts the version being shipped. Do not ask for that marker on a
  support-policy change — see
  [release-notes](../release-notes/SKILL.md) for the classification rules.
- **CI only runs Python 3.11 and 3.14, but the support range is 3.11–3.14.**
  A construct that breaks only on 3.12–3.13 passes CI. Reason about the whole
  range rather than trusting a green build.
- **`langchain-azure-compute` enforces 100% coverage** (`fail_under = 100`).
  A new uncovered branch there fails CI, so a new `if` or `except` without a
  test is a real finding in that package only.
- **`langchain-azure-ai` lazy imports must be updated in three places** — the
  `TYPE_CHECKING` import, `__all__`, and `_module_lookup`. Updating fewer makes
  the symbol import-time-invisible or `__all__`-inconsistent, and unit tests
  catch only some of these.
- **Deprecation decorators differ per package.** `azure-ai` and
  `azure-dynamic-sessions` use their own `_api.base` (`deprecated`,
  `experimental`); the other packages use `langchain_core._api` (`beta`,
  `deprecated`). Do not flag one package for using the other's convention.
- **New Azure client construction must stamp the package user agent.** Each
  package defines its own constant or helper (`USER_AGENT`, `_user_agent`,
  `get_user_agent`, `with_user_agent`). A new client path that omits it
  silently drops partner telemetry attribution.
- **`asyncio_mode = "auto"`** in every package: async tests need no
  `@pytest.mark.asyncio`. Do not ask for it.
- **`--strict-markers` and `--strict-config`** are set: a new `pytest.mark.*`
  must be registered in that package's `pyproject.toml` or collection fails.
- **`azure-cosmosdb`'s local instructions still describe `poetry`**, but its
  `Makefile` and CI use `uv run --frozen`. The `Makefile` is authoritative;
  do not flag correct `uv` usage there.
- **`azure-postgresql`'s local instructions ask for Sphinx-style docstrings**
  while its `ruff` config sets `pydocstyle` convention to `google`. Follow the
  style already used in the file being changed and raise no docstring-style
  findings in that package.
- **Unit tests must not touch the network.** `azure-ai` enforces this with
  `pytest-socket`; the same expectation applies everywhere. A new unit test that
  reaches a live service is a finding.

## Severity

Copilot code review labels comments High, Medium, or Low. Use that vocabulary.

- **High** — data loss, credential or secret exposure, a regression in released
  public API, or a failure most users of the changed path will hit.
- **Medium** — a real correctness, compatibility, or resource-handling defect
  on a narrower but supported path.
- **Low** — a genuine but minor defect worth fixing.

If a finding does not clear the Low bar, leave it out.

## Comment format

Keep each comment to the smallest useful line range and this shape:

> **[Severity] Short imperative title**
>
> What breaks, and the specific input or code path that triggers it. Which
> contract or package rule it violates. One concrete suggested fix, only when
> it is short and unambiguous.

Cite the contract by name (for example, "`VectorStore.get_by_ids` must not
raise for missing IDs") rather than linking to documentation, and prefer one
precise sentence over a paragraph of hedging.