alktunnels/tasks/tunnels/review-002-trivial-batch.md at v0.2.1

0 Fork 0

Files

T

glm-5.3-flash f7dff25f9d docs: review 002 trivial batch — D-3 honest serde test name + D-4 CHANGELOG preamble trim

- D-3: inline the serde-layer check into the renamed
  serde_rejects_unknown_substrate_value; delete the overpromising
  jsonschema_valid helper (the registry's real schema gate stays
  pinned end-to-end by the integration probes)
- D-4: trim the stale Unreleased preamble to a one-liner (written
  before the six graduation implementation commits landed)

Verified: cargo fmt --check; cargo clippy --all-targets -D warnings;
cargo test (86 + 2 doc-tests) green

2026-09-20 08:30:48 +00:00

3.5 KiB

Raw Permalink Blame History

id, name, status, depends_on, scope, risk, impact, level, tags

name

status

depends_on

scope

risk

impact

level

Description

Two trivial review 002 findings (docs/reviews/002-graduation-implementation-review.md) batched — each a one-line-scale change; no behavior impact.

D-3 — the test helper's name overpromises

src/params.rs:628-637: jsonschema_valid parses the value with serde and pattern-matches the substrate string — it validates the serde layer, NOT the JSON Schema (the registry compiles the real validators; adding a jsonschema dev-dependency purely for this unit test is not warranted). The call-site schema_rejects_unknown_substrate_value therefore promises more than it delivers; the sibling direct_target_schema_gates_shape_per_substrate is honest (it does structural schema assertions and says so).

Fix: rename jsonschema_valid → serde_valid (or inline the check) and rename the call-site test schema_rejects_unknown_substrate_value → serde_rejects_unknown_substrate_value (its doc comment should say "the serde layer; the registry's real schema gate is pinned end-to-end by the integration probes (schema_rejects_unknown_substrate_loudly, malformed_target_is_schema_class…, the forwarded listen: 0 gate)"). Do NOT add a jsonschema dev-dependency.

D-4 — the CHANGELOG "Unreleased" preamble is stale

CHANGELOG.md's Unreleased section still opens with "implementation is decomposed in tasks/tunnels/ and lands in the next release… the only behavior-affecting change so far (no wire or API change at this level yet)" — written before the six graduation implementation commits. The ### Added section below it correctly lists all six features; the preamble contradicts the section it introduces.

Fix: trim the preamble to a one-liner (e.g. "The graduation implementation (ADRs 007/008) landed — see Added below." — final wording agent's choice) or drop it.

Acceptance Criteria

The overpromising unit test/helper renamed (no jsonschema dev-dependency added)
The CHANGELOG Unreleased preamble trimmed or dropped
cargo test green (the renamed test included); clippy + fmt clean

References

docs/reviews/002-graduation-implementation-review.md §D-3, §D-4
src/params.rs:628-637 (the helper + call-site)
CHANGELOG.md (the Unreleased preamble)

Notes

Agent fills during implementation.

Implementation Notes (2026-09-20)

D-3: inlined the honest serde check directly into the renamed test serde_rejects_unknown_substrate_value (the jsonschema_valid helper deleted — it had exactly one call-site, and its matches!-with-guard body was convoluted beyond its job). The doc comment names the integration probes that pin the real registry-side gate end-to-end. No jsonschema dev-dependency added (the structural-assertion sibling direct_target_schema_gates_shape_per_ substrate is untouched and stays honest).
D-4: trimmed the preamble to the one-liner ("The graduation implementation … landed — see Added below."); the stale tasks/tunnels/ sentence and the "only behavior-affecting change" claim are gone.
Verification: cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test (86 default tests + 2 doc-tests) all green.

3.5 KiB Raw Permalink Blame History