Multi-Agent Review For Cross-Cutting Consistency#

What This Is and Why It Matters#

Cross-cutting consistency gaps are defects where each side of a paired relationship passes its own tests, but the relationship between sides silently drifts. The changed side is internally correct; the gap lives in the contract between the two sides. Standard CI (unit tests, clippy, fmt) cannot detect these gaps because no single test exercises both sides together.

This pattern has been confirmed in three incidents in libmagic-rs:

PR #211 (fix/relative-offset-evaluation): pstring anchor poisoning — bytes_consumed_with_pattern not updated alongside read_typed_value_with_pattern
PR #212 (chore/todo_cleanup): 6 silent failures found by parallel 6-agent review after 1,139 tests passed
PR #233 (fix/loader-non-utf8-magic-files): 5 critical consistency gaps found after 1,148 lib tests and just ci-check passed

The CI blind spot in PR #233 is documented: just ci-check runs cargo nextest run --workspace but does not include cargo test --doc . Rustdoc example failures are therefore invisible to the standard pre-merge check.

The Review Mechanism#

Run /pr-review-toolkit:review-pr to launch a 6-agent parallel review. The six agents and their focus areas:

Agent	Focus
`type-design-analyzer`	New variants, struct fields, public type changes
`comment-analyzer`	Docstring-vs-code divergence, stale comments
`silent-failure-hunter`	Missing error paths, swallowed errors, wrong log levels
`code-reviewer`	General correctness, cross-cutting logic gaps
`pr-test-analyzer`	Test coverage gaps, missing edge cases
`code-simplifier`	Simplification opportunities, dead code

Each agent reviews the full diff independently. All findings are consolidated into a single pass of follow-on commits. Prior incidents show 5–12 findings per run that cargo test missed entirely.

For methodology context, see docs/solutions/developer-experience/multi-agent-pr-review-fixes.md (written after PR #212) .

The Five Consistency-Partner Classes#

These are the classes of relationship drift found in PR #233. Each class is a recurring structural pattern, not a one-time bug.

1. Cross-Type Policy#

Partners: apply_equal cross-type arm in src/evaluator/operators/equality.rs ↔ compare_values cross-type arm in src/evaluator/operators/comparison.rs

Symptom: Byte-equal values compared with == return true, but both < and > return false, breaking comparison trichotomy. Equality and ordering use separate code paths; a fix to one does not propagate to the other.

2. Dual-Purpose Helper#

Partners: read_typed_value_with_pattern ↔ bytes_consumed_with_pattern — both in src/evaluator/types/mod.rs

Symptom: The read function returns the correct value; the consumed-bytes function returns a stale count, corrupting the relative-offset anchor silently. This is a recurring class: it also caused SEC-001 pstring anchor poisoning in PR #211 and the search_bytes_consumed window-vs-match-end gap documented in GOTCHAS S2.6.

3. Doc-vs-Code#

Partners: A docstring that promises a specific EvaluationError variant ↔ the actual enum (which lacks that variant), causing the helper to fall back to internal_error.

Symptom: The fallback error variant is NOT in the engine's graceful-skip allowlist (src/evaluator/engine/mod.rs). A rule that should skip silently instead propagates as an unhandled error, breaking evaluation.

4. Struct-vs-Rustdoc#

Partners: A new field added to MagicRule or OffsetSpec ↔ the # Examples rustdoc blocks that construct those structs.

Symptom: cargo test passes (unit tests use struct update syntax or builders); cargo test --doc fails because rustdoc examples are verbatim and miss the new field. Invisible to just ci-check, which does not include cargo test --doc .

5. Parse-vs-Diagnostic#

Partners: A parser tolerance branch that silently drops unrecognized or unimplemented syntax ↔ a log emission that should notify the caller.

Symptom: Users hitting an unimplemented feature see no breadcrumb. The tolerance branch exists so files load cleanly, but without a warn! or info! the missing behavior is invisible. Related: GitHub issue #47 (parse warnings silently swallowed).

If You Change X, Also Check Y#

Use this cheatsheet when reviewing or authoring a PR. If the diff touches the left column, the right column is a required consistency check.

If you change…	Also check…	Risk if missed
`apply_equal` cross-type arm	`compare_values` cross-type arm	Trichotomy violation: `==` true but `<` and `>` both false
`read_typed_value_with_pattern` pattern arm	`bytes_consumed_with_pattern` pattern arm + `calculate_default_strength`	Silent anchor corruption for relative offsets
New `EvaluationError` variant	Engine graceful-skip match arm	New error propagates as unhandled instead of graceful skip
New required field on `MagicRule` / `OffsetSpec`	Grep all `MagicRule {` rustdoc examples + run `cargo test --doc`	Rustdoc examples fail silently (not in `just ci-check`)
Parser tolerance path that drops syntax	Add `warn!` (typo / unimplemented-with-correctness-impact) or `info!` (known-unimplemented) with tracking-issue reference	Users get no breadcrumb; issue #47
New `Value` variant	GOTCHAS S2.3 (exhaustive match checklist)	Missed match arms in comparisons, formatting, serialization
New `TypeKind` variant	GOTCHAS S2.1 + `is_string_family_type` if string-family	`TypeKind::String16` was missing from `is_string_family_type` — found by PR #233 multi-agent review
New `MetaType` variant	GOTCHAS S2.11 + evaluator dispatch decision	Dispatch silently falls through to default arm

When to Run `/pr-review-toolkit:review-pr`#

Run the 6-agent review whenever a PR diff includes any of the following:

A new Value, TypeKind, or EvaluationError variant
A new required field on any public AST type (MagicRule, OffsetSpec, etc.) that has rustdoc # Examples blocks
A parser branch that consumes syntax but defers or omits full semantics (tolerance / silently-skipped directive)
Any cross-cutting refactor touching both the parser and the evaluator
A new type-reading path in src/evaluator/types/mod.rs (always verify the paired bytes_consumed path)
A new operator arm in src/evaluator/operators/ (verify all operator files that share the same type dispatch)

Complementary checks to run manually:

# Catch rustdoc example failures invisible to just ci-check
cargo test --doc

# Verify new enum variants are covered in existing matches
cargo clippy -- -D warnings