Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
10beefd
docs: add maintainer review playbook and code review checks
liu-cong Oct 26, 2026
2c8967a
docs: refine review playbook citations, code claims, and deduplication
liu-cong Oct 26, 2026
3150df5
docs: tighten review playbook pointer and public-issue rule
liu-cong Oct 27, 2026
c50f49e
docs: reconcile review playbook with maintainer debates and code
liu-cong Oct 28, 2026
d9d5684
docs: import OKF v0.2 review bundle and renderer
liu-cong Oct 30, 2026
669a430
build: gate review playbook sync in presubmit and point AGENTS.md to …
liu-cong Oct 30, 2026
0030f85
Merge remote-tracking branch 'origin/main' into review-playbook
liu-cong Nov 1, 2026
5160abc
fix(review): align GFM anchor slugger and RBAC manifest scope paths
liu-cong Nov 1, 2026
b70b6c5
docs(review): bound rule citations and add context-bounded loading gu…
liu-cong Nov 1, 2026
4fb469d
hack: detect unused footnote definitions in validate_bundle
liu-cong Nov 1, 2026
dc119ce
Merge remote-tracking branch 'origin/main' into review-playbook
liu-cong Nov 2, 2026
cd10c10
docs(review): include telemetry catalog and presubmit checks
liu-cong Nov 2, 2026
ff63033
docs(review): enforce path freshness and stale_after in verify-review…
liu-cong Nov 2, 2026
78a1c23
docs(review): drop deictic wording in maintenance playbook
liu-cong Nov 2, 2026
5607392
docs(review): split review playbook into agent rules and human guide
liu-cong Nov 2, 2026
4e8bce2
docs(review): drop deictic PR reference and use GitHub handle in meta…
liu-cong Nov 2, 2026
0c59929
docs(review): cover all Go packages in routing and harden bundle checks
liu-cong Nov 3, 2026
ca39c56
fix(review): use linear scanners for links and YAML in render_playbook
liu-cong Nov 3, 2026
b0764f6
docs(review): prune Stage-1 rules to non-code contracts with two-pass…
liu-cong Nov 4, 2026
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
9 changes: 7 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

llm-d Router. Go service that routes inference requests to model-serving pods via an Endpoint Picker (EPP), with a sidecar that coordinates disaggregated inference. Multi-vendor open source under the llm-d project. Review bandwidth is a shared community resource, so scope work tightly and discuss substantive changes in the open before code lands.

`make help` lists targets. `make presubmit` is the pre-merge gate. All targets run inside a builder container; host Go is not required.
`make help` lists targets. `make presubmit` is the pre-merge gate. Build, lint, and test targets run inside a builder container; `make tidy`, `make upgrade-deps`, and the code generation targets call the host Go toolchain.

## Agent operating rules

Expand All @@ -15,6 +15,7 @@ llm-d Router. Go service that routes inference requests to model-serving pods vi
## Working in the codebase

- Add trace attributes and span names in `pkg/common/observability/semconv/registry/` and run `make update-telemetry` to generate `llm_d.go` and `spans.go`; do not edit generated files directly.
- When a change alters a contract in [docs/review/](docs/review/index.md), moves a path in its routing table, or settles an open question there, update `docs/review/` and run `python3 hack/render_playbook.py` in the same PR (`make verify-review-playbook`). See [maintain_review_bundle.md](docs/review/playbooks/maintain_review_bundle.md).
- State your interpretation before coding. When the task has multiple valid reads, ask; don't pick one silently. For clear failure signals (logs, failing tests, reproducer), act; the ask rule is about unclear requirements, not unclear bugs.
- Define success as a checkable outcome: "add validation" becomes "write failing tests for invalid inputs, then make them pass". Where the issue is reproducible, the failing test IS the success criterion; write it first and let it gate the implementation.
- Before changing or extending a component, read an analogous one in the repository. The closest existing implementation is the canonical pattern; follow its structure, naming, and tests rather than introducing new conventions.
Expand All @@ -28,6 +29,9 @@ llm-d Router. Go service that routes inference requests to model-serving pods vi
## Code review

- Check every constant the change adds: grep its value across the repository and report any that already exists under another name for the same contract, any that declares a shared contract's value locally rather than importing it, and any declared away from its only reader.
- Execute a two-pass additive review per [review_a_pull_request.md](docs/review/playbooks/review_a_pull_request.md):
- **Pass 1 (Code and full-repo search first):** Inspect the diff, read modified files in full, and search the repository (`rg`/`grep` callers, sibling implementations, `pkg/common/`, `config/charts/`, tests, and docs) to catch logic bugs, edge cases, concurrency races, and contract mismatches directly from source code. Record all verified Pass-1 code findings before opening any Stage-1 rule file.
- **Pass 2 (Targeted non-code decisions - supplement only):** Route changed paths through [review_a_pull_request.md](docs/review/playbooks/review_a_pull_request.md) and load only the `0` to `2` matching rule files under `docs/review/rules/` (`0` rule files for test-only, doc-only, build, or CI diffs; never load `docs/review_playbook.md` or `docs/review/provenance.json`). Apply a rule only when its `When:` condition appears in the diff; Stage-1 rules are strictly additive to Pass 1, and matching `0` `When:` triggers must never drop, suppress, or downgrade verified Pass-1 code, concurrency, caller/callee, or test defects. Write natural code-focused review comments without citing internal `docs/review/` rule IDs.

## Pull requests

Expand Down Expand Up @@ -76,7 +80,8 @@ The codebase uses `go-logr` via controller-runtime. Verbosity constants are defi

**Level conventions:**

- `logger.Info(...)` for once-per-request operational signals.
- `logger.Info(...)` for one-time startup or lifecycle signals.
- `logger.V(logging.DEFAULT).Info(...)` for once-per-request operational signals.
- `logger.V(logging.DEBUG).Info(...)` for per-item or per-loop signals that fire multiple times per request.
- `logger.V(logging.TRACE).Info(...)` for detailed state transitions (cache operations, index updates).
- `logger.Error(err, "msg", ...)` for recoverable errors that carry an underlying `error` value.
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Please access the
[llm-d organization contributing guide](https://github.com/llm-d/llm-d/blob/main/CONTRIBUTING.md) for overall contributing guidelines. This document covers only what is different or specific to this repository.

See [DEVELOPMENT.md](DEVELOPMENT.md) for how to build, run, and test the codebase.
See [DEVELOPMENT.md](DEVELOPMENT.md) for how to build, run, and test the codebase, [docs/review_playbook.md](docs/review_playbook.md) for the compiled human-readable review guide, and [AGENTS.md](AGENTS.md) when working with coding or review agents.

## Claiming an issue

Expand Down
5 changes: 2 additions & 3 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,7 @@ If using coding agents, please ensure that the agent uses the PR template format
The template contains a `release-notes` section which must be filled for any change that has
user facing impact; a required check validates it on the PR.

For additional information and context, please refer to the [llm-d contributing guide](https://github.com/llm-d/llm-d/blob/main/CONTRIBUTING.md)
For additional information and context, please refer to the [llm-d contributing guide](https://github.com/llm-d/llm-d/blob/main/CONTRIBUTING.md), the compiled human [review playbook](docs/review_playbook.md), and the source review bundle in [docs/review/](docs/review/index.md).

### Presubmit

Expand All @@ -992,5 +992,4 @@ Before opening a PR, run:
make presubmit
```

This runs the same lint, vet, and test checks as the CI pipeline. Fixing failures locally
saves a round-trip through GitHub Actions.
This runs the same lint, vet, test, and review-playbook checks as the CI pipeline (`make verify-review-playbook` verifies `docs/review_playbook.md` against `docs/review/`). Fixing failures locally saves a round-trip through GitHub Actions.
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ check-latest-tags-strict: ## Check ':latest' image tags in YAML (strict; fails o

.PHONY: presubmit
presubmit: LINT_NEW_ONLY=true
presubmit: git-branch-check signed-commits-check go-mod-check format lint lint-security vulncheck check-latest-tags-strict verify-boilerplate verify-telemetry
presubmit: git-branch-check signed-commits-check go-mod-check format lint lint-security vulncheck check-latest-tags-strict verify-boilerplate verify-telemetry verify-review-playbook

.PHONY: update-telemetry verify-telemetry test-telemetry
update-telemetry: image-build-builder ## Generate trace attributes, span names, and the catalog.
Expand Down Expand Up @@ -375,6 +375,10 @@ verify-manifests: kubectl-validate ## Validate deployment manifests.
verify-boilerplate: ## Check that .go and .sh files carry a copyright notice.
hack/copyright.sh verify

.PHONY: verify-review-playbook
verify-review-playbook: image-build-builder ## Check that docs/review/ is valid and docs/review_playbook.md is in sync.
$(BUILDER_RUN) 'python3 hack/render_playbook.py --check'

##@ Helm

.PHONY: verify-helm-charts
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
type: Decision
title: "Channel-fed maintenance loop with lock protection for dynamic priority bands"
description: "Dynamic InferenceObjective priority bands provision off the hot path via edge-triggered controller reconciliation feeding a lock-protected processor loop."
tags: ["maintainer-vs-maintainer", "mvm-10"]
status: stable
sources:
- { id: "issue-1184", resource: "https://github.com/llm-d/llm-d-router/issues/1184", author: "human:LukeAVanDrie", last_modified: "2026-05-18T06:13:38Z" }
- { id: "issue-1184-ic-4477880575", resource: "https://github.com/llm-d/llm-d-router/issues/1184#issuecomment-4477880575", author: "human:shmuelk", last_modified: "2026-05-18T12:55:40Z" }
---

# Context

Maintainers compared reconciling dynamic flow-control priority bands directly under registry locks against driving band provisioning through a single-owner processor channel. (Conflict audit ID: `MVM-10`.)

# Positions considered

- **Position A (human:LukeAVanDrie)**: Proposed in #1184.[^issue-1184]
- **Position B (human:shmuelk)**: Counter-position evaluated in #1184.[^issue-1184-ic-4477880575]

# Outcome

Merged edge-triggered controller reconciliation feeding the processor loop with read/write lock protection and idle drain before removal.

[^issue-1184]: [#1184](https://github.com/llm-d/llm-d-router/issues/1184) (human:LukeAVanDrie)
[^issue-1184-ic-4477880575]: [#1184](https://github.com/llm-d/llm-d-router/issues/1184#issuecomment-4477880575) (human:shmuelk)
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
type: Decision
title: Delete stranded deprecation helpers in the removal pull request
description: Unreferenced registration and compatibility helpers are deleted in the pull request that removes their last caller rather than retained for hypothetical reuse.
tags: ["maintainer-vs-maintainer", "mvm-12"]
status: stable
sources:
- { id: "pr-2509-ic-5398059091", resource: "https://github.com/llm-d/llm-d-router/pull/2509#issuecomment-5398059091", author: "human:roytman", last_modified: "2026-08-24T16:16:22Z" }
- { id: "pr-2509-ic-5398602893", resource: "https://github.com/llm-d/llm-d-router/pull/2509#issuecomment-5398602893", author: "human:elevran", last_modified: "2026-08-24T17:03:39Z" }
---

# Context

When removing expired deprecated plugin aliases, authors offered to keep `RegisterDeprecatedWithPluginDependencies` for future deprecations versus deleting it immediately. (Conflict audit ID: `MVM-12`.)

# Positions considered

- **Position A (human:roytman)**: Proposed in #2509.[^pr-2509-ic-5398059091]
- **Position B (human:elevran)**: Counter-position evaluated in #2509.[^pr-2509-ic-5398602893]

# Outcome

Deleted unreferenced deprecation helpers in the stranding change, relying on version control history if needed later.

Related rule: [N+2 deprecation for shipped surfaces and zero shims for unshipped surfaces](../rules/governance_and_compatibility_policy.md#n2-deprecation-for-shipped-surfaces-and-zero-shims-for-unshipped-surfaces).

[^pr-2509-ic-5398059091]: [#2509](https://github.com/llm-d/llm-d-router/pull/2509#issuecomment-5398059091) (human:roytman)
[^pr-2509-ic-5398602893]: [#2509](https://github.com/llm-d/llm-d-router/pull/2509#issuecomment-5398602893) (human:elevran)
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
type: Decision
title: "Token count estimation encapsulated inside token-producer"
description: "Fast byte-length and tokenized prompt estimates live inside token-producer rather than as methods on core InferenceRequest types."
tags: ["maintainer-vs-maintainer", "mvm-15"]
status: stable
sources:
- { id: "pr-1438-r3340589325", resource: "https://github.com/llm-d/llm-d-router/pull/1438#discussion_r3340589325", author: "human:ahg-g", last_modified: "2026-06-02T10:47:20Z" }
- { id: "pr-1438-r3340290186", resource: "https://github.com/llm-d/llm-d-router/pull/1438#discussion_r3340290186", author: "human:vMaroon", last_modified: "2026-06-02T09:56:40Z" }
---

# Context

Maintainers debated exposing token estimation directly on `InferenceRequest` to skip tokenization versus encapsulating all token logic inside `token-producer`. (Conflict audit ID: `MVM-15`.)

# Positions considered

- **Position A (human:ahg-g)**: Proposed in #1438.[^pr-1438-r3340589325]
- **Position B (human:vMaroon)**: Counter-position evaluated in #1438.[^pr-1438-r3340290186]

# Outcome

Unified tokenization and fast token estimation inside `token-producer` so downstream scorers consume one attribute contract.

[^pr-1438-r3340589325]: [#1438](https://github.com/llm-d/llm-d-router/pull/1438#discussion_r3340589325) (human:ahg-g)
[^pr-1438-r3340290186]: [#1438](https://github.com/llm-d/llm-d-router/pull/1438#discussion_r3340290186) (human:vMaroon)
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
type: Decision
title: Core endpoint identity on EndpointMetadata and optional topology in AttributeMap
description: "Core identity fields live on EndpointMetadata while optional per-endpoint properties such as worker rank flow through datalayer AttributeMap."
tags: ["maintainer-vs-maintainer", "mvm-06"]
status: stable
sources:
- { id: "pr-1106-ic-4478669785", resource: "https://github.com/llm-d/llm-d-router/pull/1106#issuecomment-4478669785", author: "human:elevran", last_modified: "2026-05-18T14:27:01Z" }
- { id: "pr-1106-r3271256891", resource: "https://github.com/llm-d/llm-d-router/pull/1106#discussion_r3271256891", author: "human:ahg-g", last_modified: "2026-05-20T04:41:17Z" }
---

# Context

Maintainers debated placing endpoint rank and type directly on `EndpointMetadata` versus keeping `EndpointMetadata` minimal and storing optional properties in `Attributes`. (Conflict audit ID: `MVM-06`.)

# Positions considered

- **Position A (human:elevran)**: Proposed in #1106.[^pr-1106-ic-4478669785]
- **Position B (human:ahg-g)**: Counter-position evaluated in #1106.[^pr-1106-r3271256891]

# Outcome

`EndpointMetadata` carries core identity (`ID`, `Name`, `Type`), while worker rank and optional topology signals flow through data-layer attributes.

[^pr-1106-ic-4478669785]: [#1106](https://github.com/llm-d/llm-d-router/pull/1106#issuecomment-4478669785) (human:elevran)
[^pr-1106-r3271256891]: [#1106](https://github.com/llm-d/llm-d-router/pull/1106#discussion_r3271256891) (human:ahg-g)
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
type: Decision
title: "Flow control remains opt-in by default until standalone default-on graduation"
description: "Flow control ships with GA code maturity while remaining gated off by default so baseline deployments incur zero queueing change without explicit opt-in."
tags: ["maintainer-vs-maintainer", "mvm-23"]
status: stable
sources:
- { id: "pr-2180-ic-5119639333", resource: "https://github.com/llm-d/llm-d-router/pull/2180#issuecomment-5119639333", author: "human:ahg-g", last_modified: "2026-07-29T15:04:01Z" }
- { id: "pr-2180-r3692815708", resource: "https://github.com/llm-d/llm-d-router/pull/2180#discussion_r3692815708", author: "human:liu-cong", last_modified: "2026-07-31T19:14:37Z" }
---

# Context

Maintainers debated enabling flow control by default upon GA code maturity versus keeping the feature opt-in until default-on operational readiness is verified separately. (Conflict audit ID: `MVM-23`.)

# Positions considered

- **Position A (human:ahg-g)**: Proposed in #2180.[^pr-2180-ic-5119639333]
- **Position B (human:liu-cong)**: Counter-position evaluated in #2180.[^pr-2180-r3692815708]

# Outcome

Merged GA stability improvements with the `flowControl` gate off by default, deferring default-on activation to a dedicated graduation issue.

[^pr-2180-ic-5119639333]: [#2180](https://github.com/llm-d/llm-d-router/pull/2180#issuecomment-5119639333) (human:ahg-g)
[^pr-2180-r3692815708]: [#2180](https://github.com/llm-d/llm-d-router/pull/2180#discussion_r3692815708) (human:liu-cong)
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
type: Decision
title: GitHub CI checks are the source of truth for commit requirements
description: "Enforced GitHub CI workflows define commit sign-off and formatting requirements rather than duplicate prose rules in contributor docs."
tags: ["scope-and-process", ci, commits]
status: stable
verified: { by: "human:liu-cong", at: "2026-10-29T00:00:00Z" }
---

# Context

Contributor documentation and CI scripts (`scripts/check-commits.sh` and GitHub status checks) risked drifting when commit trailers or formatting rules changed.

# Tradeoff

Maintaining separate prose lists of commit rules across `AGENTS.md`, `CONTRIBUTING.md`, and `DEVELOPMENT.md` creates contradictions whenever workflow checks evolve.

# Outcome

GitHub CI checks are the canonical source of truth for commit requirements (including DCO sign-off via `git commit -s`); contributor docs point to enforced checks rather than restating separate rules.

Related rule: [AGENTS.md Git workflow](../../../AGENTS.md#git-workflow).
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
type: Decision
title: "Helm charts for end-user releases and deploy Kustomize manifests for local kind and E2E"
description: "End-user deployments ship through config/charts Helm charts while Kustomize overlays under deploy remain scoped to local development and E2E suites."
tags: ["maintainer-vs-maintainer", "mvm-09"]
status: stable
sources:
- { id: "issue-1177", resource: "https://github.com/llm-d/llm-d-router/issues/1177", author: "human:ahg-g", last_modified: "2026-05-17T21:01:28Z" }
- { id: "issue-1177-ic-4478503514", resource: "https://github.com/llm-d/llm-d-router/issues/1177#issuecomment-4478503514", author: "human:shmuelk", last_modified: "2026-05-18T14:08:19Z" }
---

# Context

Maintainers discussed deleting `deploy/` Kustomize manifests entirely in favor of Helm guides versus retaining `deploy/` for local `kind` environments and E2E suites. (Conflict audit ID: `MVM-09`.)

# Positions considered

- **Position A (human:ahg-g)**: Proposed in #1177.[^issue-1177]
- **Position B (human:shmuelk)**: Counter-position evaluated in #1177.[^issue-1177-ic-4478503514]

# Outcome

Kept `config/charts/` as the canonical end-user packaging surface while retaining `deploy/` for local `kind` workflows and E2E tests.

Related rule: [Helm charts in config/charts/ and local Kustomize in deploy/](../rules/governance_and_compatibility_policy.md#helm-charts-in-configcharts-and-local-kustomize-in-deploy).

[^issue-1177]: [#1177](https://github.com/llm-d/llm-d-router/issues/1177) (human:ahg-g)
[^issue-1177-ic-4478503514]: [#1177](https://github.com/llm-d/llm-d-router/issues/1177#issuecomment-4478503514) (human:shmuelk)
Loading
Loading