Skip to content

translator-rules: codify recurring auto-review patterns #6

@a2chang

Description

@a2chang

Update docs/translation-rules.md with the patterns that surfaced repeatedly during the INQCUST POC auto-review (PR #4, 6 rounds). The companion code-side issue is #5.

Goal: every subsequent program PR should converge in ≤2 review rounds because the rulebook is now explicit on these points.

Patterns to codify

  1. Single 500 JSON schema via ProblemDetail end-to-end. Successful responses are domain DTOs; every error response (4xx/5xx) is ProblemDetail (RFC 7807). No per-program legacy result-record leaks to the wire on the error path. @RestControllerAdvice builds the body; controllers do not handcraft error JSON.

  2. Defensive null-checks at service-method boundaries. Every public service method begins with Objects.requireNonNull(arg, "arg must not be null") for each parameter. Every constructor of an immutable result record (e.g. …Result.success(record)) null-checks its arguments before assigning fields.

  3. @Valid on controller request bodies. Any controller method that accepts a @RequestBody DTO must mark the parameter @Valid. Validation annotations on DTO classes that are never @Valid-bound are dead code and must be removed or wired up.

  4. Deterministic randomness via injected source / clock. Services must not call ThreadLocalRandom, Math.random, Random.next*, or Instant.now() directly. Inject a small interface (e.g. RandomCustomerNumberGenerator, Clock) and provide a deterministic test double. Integration tests must not depend on probabilistic outcomes.

  5. Retry off-by-one: random-exhaustion vs not-found. When a bounded retry loop in random-pick mode exhausts without finding a target, the response is a distinct "retry exhausted" outcome (fail code "R" → HTTP 503), not a generic backend abend. "Exact key not found" remains fail code "1" → HTTP 404.

  6. Control-baseline preservation on writes and in tests. The control table holds Flyway-seeded invariants (sortcode, last_*_number, *_count). Production code must UPDATE rather than DELETE+insert; integration-test cleanup must either skip control entirely (preferred) or re-insert the canonical baseline row after deleting. The canonical baseline row is sortcode 987654, all counters/last-numbers 0.

  7. Empty-table → 404, not 500. For read paths, an empty target table is semantically "not found", matching the COBOL behavior where an empty file produces the same fail code as a missing record. Map empty-table outcomes to the regular not-found fail code, not the backend-failure code.

Acceptance

  • docs/translation-rules.md has new sections covering each of the seven patterns above, with a brief rationale and a code-shape example for each.
  • The rulebook references PR feat(inqcust): translate INQCUST to Java #4 as the empirical source.
  • Existing rules are not removed; only added to.
  • A short "checklist for translators" appendix at the bottom of the doc summarizes the seven items as bullet points so workers can scan them quickly.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions