Compare commits

...
Author SHA1 Message Date
Hongsheng Liu 48bd050974 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

No files matched your search

+8 -1
View File
@@ -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 |