docs(review): enforce configuration provenance

Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
2026-08-09 22:39:57 +08:00
6 changed files with 74 additions and 5 deletions
@@ -94,6 +94,7 @@ If docs and live code disagree, verify the code/tests and report the drift.
 | --- | --- |
 | [model-addition-checklist.md](references/checks/model-addition-checklist.md) | A model, architecture, loader, processor, registry, or stage config is added. |
 | [perf-verification.md](references/checks/perf-verification.md) | The PR makes a latency, throughput, memory, or quality claim. |
+| [config-provenance.md](references/checks/config-provenance.md) | Changed or newly load-bearing behavior reads config, CLI, environment, legacy, or default values. |
 | [test-quality-evaluation.md](references/checks/test-quality-evaluation.md) | Tests change, are absent for risky code, or may not exercise production behavior. |
 | [tests-docs-checklist.md](references/checks/tests-docs-checklist.md) | Coverage, CI markers, examples, user docs, or PR evidence need review. |
 | [verification.md](references/checks/verification.md) | Hardware, a server, or a runnable affected path is available for active verification. |
@@ -152,13 +153,19 @@ contract they protect or use only the applicable evidence checks.
 Apply every category in [general-checks.md](references/process/general-checks.md) before
 lower-priority comments.

-For each changed value or behavior, trace:
+For each changed or newly load-bearing value or behavior, trace:

 ```text
 public ingress -> validation/defaulting -> producer -> transformations
  -> stage/worker/connector boundary -> final consumer -> terminal cleanup
 ```

+Treat unchanged context as in scope when the diff makes it authoritative for a
+new consumer. When config, CLI, environment, legacy, or default sources can
+control that value, apply
+[config-provenance.md](references/checks/config-provenance.md) and prove the
+documented source at the final sink with distinct sentinel values.
+
 Cover every applicable offline/online, streaming/non-streaming, sync/async,
 feature-on/off, topology, and compatibility path. Search bounded callers and
 sibling implementations rather than assuming the changed hunk is the only path.
@@ -0,0 +1,49 @@
+# Configuration Provenance
+
+Read this reference when changed or newly load-bearing behavior reads config,
+CLI, environment, legacy, alias, or default values. Always use it for endpoint,
+host, port, address, connector metadata, and `*_extra_config` consumers, even
+when the schema or documentation is unchanged.
+
+## Trace the source of truth
+
+For each semantic value, record one compact ledger:
+
+```text
+documented input -> parsed field -> competing sources and precedence
+  -> runtime projection/transport -> final consumer
+```
+
+- Search the exact field, environment variable, default literal, and aliases in
+  code, tests, docs, examples, and deployment files.
+- Treat unchanged producers and context lines as review scope when the diff
+  starts relying on them or makes their output authoritative.
+- Resolve the intended precedence from active branch-local docs, enforced
+  code/tests, and compatibility policy. Report a documented input that is
+  silently ignored or reinterpreted at a live sink.
+- Check every supported construction path and feature-off behavior without
+  inventing parity for unsupported paths.
+
+## Require discriminating evidence
+
+Give every competing source a distinct value so the test reveals which source
+wins. Prefer non-default sentinels; for example:
+
+```text
+documented config port = 25301
+environment/default port = 8998
+expected final endpoint = http://10.0.0.8:25301
+```
+
+Assert the final URL, connection target, runtime argument, or other live sink,
+not only an intermediate object. A test that changes the environment value to
+the expected configured value proves formatting, not configuration propagation.
+For a bug fix, show the discriminating test fails on the frozen base and passes
+on the reviewed head when the environment permits execution.
+
+## Finding bar
+
+A supported non-default value reaching the wrong endpoint, device, topology, or
+runtime behavior is a correctness or compatibility finding. A missing
+discriminating test alone is a validation gap unless repository policy requires
+that regression coverage for the change.
@@ -16,6 +16,9 @@ were reverted or broken. Check:
  instead of the changed behavior being mocked away;
 - fakes preserve relevant types, MRO, shapes, devices, async behavior, and
  lifecycle transitions;
+- configuration tests give competing config, CLI, environment, legacy, and
+  default sources distinct non-default values and assert the final consumer;
+  setting only the source read by the implementation does not prove precedence;
 - seeds, ordering, timing, synchronization, external services, and numeric
  tolerances are deterministic or explicitly controlled;
 - normal, invalid, boundary, feature-off, failure/cancellation, and regression
@@ -15,6 +15,9 @@ and [design documents](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/
 - Trace each changed value through public ingress, validation/defaulting,
  producer, transformations, process or device boundaries, final consumer, and
  terminal cleanup.
+- Treat unchanged code as in scope when the diff makes its value newly
+  load-bearing for a changed consumer; validate that value's origin and contract
+  rather than reviewing only added lines.
 - Check every applicable sync/async, offline/online, streaming/non-streaming,
  feature-off, topology, and compatibility path without demanding unsupported
  modes merely for symmetry.
@@ -123,7 +126,9 @@ distinct live caller, invariant, or compatibility need.

 ## Finding bar

-Anchor each finding to a changed `path:line`; name the trigger or call path,
-current behavior, impact, and smallest fix direction. Treat pending CI, missing
-hardware, or unsupported measurements as validation gaps unless the repository
-contract makes that evidence a merge requirement.
+Anchor each finding to the exact `path:line` that causes the behavior; it may be
+unchanged when the diff makes it newly load-bearing, in which case name the
+changed call path that brings it into scope. Include the trigger, impact, and
+smallest fix direction. Treat pending CI, missing hardware, or unsupported
+measurements as validation gaps unless the repository contract makes that
+evidence a merge requirement.
@@ -166,6 +166,10 @@ index patch, worktree patch, and untracked-content manifest. Any mismatch makes
 the review stale; discard the affected evidence and restart. Prefer one
 root-cause comment to several symptom comments.

+When the root cause is an unchanged line made newly load-bearing by the diff,
+cite it in the consolidated review body instead of moving the comment to an
+unrelated changed line merely to make it inline.
+
 Local presentation is the default. Only explicit authorization permits GitHub
 posting. When authorized, publish one consolidated final review after
 validation; do not post preliminary or incremental comments. Submit a review
@@ -50,6 +50,7 @@ compatibility changes.
 | --- | --- | --- |
 | New or expanded model, loader, processor, registry, or stage config | [model-addition-checklist.md](../checks/model-addition-checklist.md) | [`add-tts-model`](../../../add-tts-model/SKILL.md) or [`add-diffusion-model`](../../../add-diffusion-model/SKILL.md) |
 | Latency, throughput, memory, scaling, precision, or quality claim | [perf-verification.md](../checks/perf-verification.md) | [`diffusion-perf-opt`](../../../diffusion-perf-opt/SKILL.md) for diffusion |
+| Config/CLI/environment/default precedence; endpoint, host, port, address, connector metadata, or `*_extra_config` | [config-provenance.md](../checks/config-provenance.md) | None |
 | Tests changed, absent for risky behavior, or test-only | [test-quality-evaluation.md](../checks/test-quality-evaluation.md) | [`vllm-omni-test`](../../../vllm-omni-test/SKILL.md) |
 | CI, examples, docs, public behavior, or contributor evidence | [tests-docs-checklist.md](../checks/tests-docs-checklist.md) | None |
 | Suitable hardware/server and runnable affected path | [verification.md](../checks/verification.md) | None |