MemberJunction / MemberJunction/MJ

CodeGen prints "Integrity check FAILED" for a condition that fails nothing, sending readers after non-issues

Open
#4,386 0 comments 0 reactions 1 assignee Claimed by @SDesai-BC View on GitHub
bug priority: low
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

Cosmetic, but it cost real time during the v6.1.0-edge.6 release and will do so again.

## What it looks like

During `codegen-idempotency-check.mjs --stage single-column`, the log prints:

```
Integrity check FAILED: entityFieldsSequenceCheck - Entity MJ: Entities has a missing sequence number. Expected 74, but found 75 for field CodeName
Entity MJ: Entities has a missing sequence number. Expected 75, but found 76 for field ClassName
... (7 lines)

✔ System integrity checks completed
✔ MJ CodeGen complete — 383 entities in 2.30s
{"version":"1","success":true,"command":"codegen", ... }
```

**`FAILED` in red, then `✔ completed` and `success: true` two lines later.** Nothing failed. The stage had already reported `✅ Stage single-column assertions PASSED` *before* this, and the run exits 0.

## Why it is benign

The single-column stage adds a probe column to `MJ: Entities`, asserts minimal blast radius, then removes it. This message comes from the **cleanup run afterwards**, when the probe column that occupied slot 74 has just been dropped and nothing has renumbered yet.

The fingerprint confirms it — every gap is off by exactly one, consecutively: `74→75`, `75→76`, … `80→81`. That is one removed row, not disorder. And the cleanup runs `codegen --skipdb --no-ai`, so CodeGen is explicitly forbidden from touching the database, which is where renumbering happens.

Steady state is clean. Checked for `Sequence <> ROW_NUMBER()` on two freshly built databases: zero rows in each; `MJ: Entities` has 80 fields, max sequence 80, fully contiguous.

## Why it is worth fixing anyway

A red `FAILED` that fails nothing trains people to ignore integrity output — and this one appears in the **drift gate's own log on every release**, right next to output that genuinely matters. During this release it prompted "why is sequence order not in order, is this concerning?", and answering it properly took a database query, a log re-read, and a walk through the stage's lifecycle. The next person pays that cost again.

## Suggested fix

Either:

- **Downgrade the wording** when the check is advisory — `Integrity check WARNING:` or `note:` — so severity matches consequence; or
- **Suppress `entityFieldsSequenceCheck` during probe cleanup**, since a transient gap is the expected state there and the check cannot renumber under `--skipdb` anyway; or
- **Say why it cannot act** — e.g. `entityFieldsSequenceCheck: 1 gap (renumber skipped: --skipdb)`.

The third is probably best: it keeps the signal, and the parenthetical is exactly the fact a reader needs to stop worrying.

## Repro

```bash
DB_DATABASE= MJ_CODEGEN_NO_AI=1 \
node scripts/codegen-idempotency-check.mjs --stage single-column --no-ai
# stage PASSES and exits 0; the cleanup run prints "Integrity check FAILED"
```

## Acceptance

- A run that exits 0 with everything passing does not print `FAILED`
- Genuine integrity failures remain clearly visible

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.