[Mathih13]

A contributor's first pull request could fail for reasons unrelated to its behaviour. About 250 tests asserted that a function body contains a literal string, and three harnesses pinned a production adapter's source verbatim. A rename or a reformat turned CI red, and the only fix was to re-bless text. Comments also carried review narration, slice and phase labels, and bare work-item numbers that do not resolve.

This is phase 1 of #635. It changes no gameplay behaviour.

Tests

• Deleted 255 test functions that scanned source text or pinned an adapter's source, in the Module and the Gateway.
• Kept the Architecture Tests that protect an invariant: Character-owned table coverage, no spatial full scan and no gameplay read of the shard id, the transfer fence chokepoint, the gc reaper's TTL tables, packed grid cells, publish safety, and binding parity. Each failure message names its invariant, and each has a negative test that shows it fires.
• The shared body scanner moved to lyracore-test-support, so the Module and Gateway no longer carry two copies.
• The print-macro ban is a CI grep in the fmt job. The panic ban at untrusted-input boundaries is a clippy lint.
• Wiring facts that a scan used to guard now have durable behaviour tests:
  • operator_gates: every gated reducer in the published schema (225 today) refuses an anonymous caller. A new reducer is covered automatically. This replaces mail_operator.
  • transfer_fence: login refuses a Character in escrow, background passes leave it unchanged, and the Instance reaper holds its Instance.
  • corpse_loot_residue, diminishing_returns, instance_party: corpse-loot purge, diminishing returns only after a placed aura, and solo Instance adoption by a new party.
  • The Gateway provision path has a test through the existing CLI seam.
  • Each new test failed when its guard was removed.

Operations

• debug_repair_after_publish moved to an ungated operations module. The reducer name and arguments are unchanged, and it still requires the Operator. Production is unchanged, because every publish already builds with debug_reducers.
• The debug module no longer claims that production publishes leave debug_reducers off.

Comments

• Removed review narration, "this PR" and "formerly" history, slice and phase labels, and about 200 bare work-item numbers. Comments that carried an invariant now state it in the present tense.
• Module docs longer than about sixty lines moved to docs/ (for example docs/module-build.md, docs/character-transfer.md, docs/runtime-script-host.md).
• Spell ids, skill lines, DBC columns and other game data numbers stay.

Local gates: fmt, clippy with -D warnings, the five crate test commands and the wasm check pass. The new durable targets pass locally.

🤖 Generated with Claude Code (Claude Opus 5.5 lead, with Opus 5.5 and Sonnet 5.5 subagents; part of the comment sweep by GPT-6.1 Sol via Codex)

[Mathih13]

Code review: Standards and Spec

Reviewed at 67d87c7 against main (merge-base 9088335). The spec is Phase 1 of #635. The two axes are reported separately and are not ranked against each other. Priority: P1 fix before merge, P2 should fix, P3 nit or judgement call.

Standards

Hard violations (documented rules)

• P1. Test gap. module/tests/operator_gates.rs:15: OPEN_PREFIX = "debug_" exempts every debug_* reducer. That exemption includes debug_repair_after_publish, which this PR moves out of the debug feature and which calls require_operator (module/src/operations.rs:27-28). The new gate test never checks the one production-critical debug-named reducer. Add it to the gated set explicitly, or narrow the exemption.
• P2. Stale cross-references (CODING_STANDARDS: "Stale explanation is a defect"):
  • module/src/tripwires.rs:1144,1153 and module/src/transfer/mod.rs:1154 point at "the verdict table in transfer/mod.rs's module doc". That doc is now one line, and docs/character-transfer.md has no table.
  • tripwires.rs:1348-1349 quotes transport.rs text that this PR deleted.
  • module/src/test_scan.rs:8-9 describes raw_table_reads but sits above on_comment_line.
  • docs/package-delta-format.md is rustdoc moved over without edits. It opens with a fragment, says "This library" and "The crate", cites the bare ids and script modules, and has five H1 headings. docs/runtime-script-host.md has the same heading problem.
• P2. Glossary Avoid words in new prose (AGENTS.md "Words", CONTEXT.md Shard: avoid "database" when it means a Shard):
  • docs/character-transfer.md: "same-database Transfer", "Across databases".
  • docs/realm-loot-routing.md: "another database", "only one database votes".
  • tripwires.rs:1290: "cross-database".
  • docs/actor-verbs.md: "rejected" for a Refusal.
• P3. Unslop. Em dashes stay in rewritten lines, for example operations.rs:246. The "ratchet" metaphor stays at tripwires.rs:1145.

Judgement calls (smells)

• P3. Possible Duplicated Code:
  • instance_of appears in both module/tests/instance_party.rs:53 and transfer_fence.rs:165.
  • The publish → claim_operator → install_guid_range setup repeats in transfer_fence.rs, mount.rs and item_guids.rs, although module/tests/support/mod.rs exists.
  • The quote-tracking loop appears in both source_scan.rs:29 and test_scan.rs:19.
  • The same 12-line deny(clippy::unwrap_used, …) block is copied into 4 Gateway files.
• P3. Possible Mysterious Name. In operator_gates.rs:50, open holds the reducers that wrongly let a caller through, and the name clashes with OPEN. ungated would say what it holds.
• P3. gateway/tests/startup_relays.rs:39 names a parameter probe (an Avoid word). Three new test names each join two behaviours (CODING_STANDARDS: one test covers one behaviour), for example tame_dismiss_and_retame_preserve_…_and_clean_live_state.
• P3. gateway/src/world/trade_tests.rs:180-190 keeps "AC:" labels whose issue is gone.

Spec

Missing or partial

• P2. "Remove … 'formerly' history, bare work-item numbers". The sweep did not finish:

  • module/src/debug/mod.rs:672 ("drives for 224").
  • "previously" narration at gateway/src/codec/item.rs:201, codec/quest.rs:51,55, codec/tests.rs:1675,2457, config.rs:785, and world/loot.rs:97-98 ("the class of bug the group slice fixed").
  • Outside Rust: gateway/Cargo.toml:36 (docs(triage): record issue outcomes and current assignments #451), importer/scripts/import-world.sh (6 refs), import-world-sql-test.sh (3), lyracore:61 (packages CLI: resolve bare names from the Official Package Collection #302).

  If the PR scopes the sweep to Rust on purpose, say so in the description.

• P2. "Keep the Architecture Tests that protect an invariant". Text pins that are not on the keep list survive:

  • tripwires.rs:1910 core_source_names_no_official_package.
  • publish_safety.rs:423 pins strings in lyracore.
  • publish_safety.rs:661,690 pin scripts/publish-module.sh, which is not in the tree, so both tests skip silently and are dead. Delete them, or make them fail when the file is missing.

• P3. "include_str! scans over … subscription sources". gateway/tests/schema_parity.rs:1254 still scans connection.rs for SELECT * FROM. It may count as the binding parity guard. If it does, name it that way.

• P3. module/src/package_test.rs:10 still re-exports code_of/shape_of to Packages, so the text-pinning API stays public. No Package uses it.

Not asked for

• P3. The clippy panic lint at untrusted-input boundaries (b2e64bc). The spec moved only the print ban. It is reasonable, but it is new scope.
• Commit 8f775a0 fixes a regression that this PR introduced. The final subscription queries are identical to main, so there is no net change.

Looks done, but wrong

• P2. "Keep present-tense invariants". Commit d30492a removed the indentation from about 101 chained-method lines in 16 files, for example gateway/src/world/tests.rs:7281-7286 (.lock() / .unwrap() at column 0). Other files include realm_core.rs, connection.rs, party_tests.rs and module/src/group.rs. rustfmt skips these macro bodies, so cargo fmt --check passes and CI will not catch it.

Confirmed done:

• No code_of(...).contains assertions remain, and the three pins of verbatim adapter source are gone.
• The issue-reference ban and the dead-code allowance rule are deleted.
• The print ban is a CI grep that fails closed.
• Each kept Architecture Test has a negative test.
• No module doc is longer than 60 lines.
• The crate-root SpacetimeDB claim matches the pin.
• The debug-module claims are fixed.
• debug_repair_after_publish keeps its name, its arguments and its Operator gate.

---

Summary:

• Standards: 4 hard findings and 6 judgement calls. Worst: operator_gates exempts debug_repair_after_publish through the debug_ prefix (P1).
• Spec: 7 findings. Worst: the comment sweep is incomplete, and indentation was stripped from about 101 lines that CI will not flag (P2).

---

Generated by Claude Code