MemberJunction / MemberJunction/MJ

No unit test asserts that CodeGen loads persisted validators under --no-ai — the same regression has shipped twice

Open Beginner friendly
#4,375 0 comments 0 reactions 0 assignees View on GitHub
chore codegen-repair
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

## The gap

`mj codegen --no-ai` silently deleting every committed `Validate()` override has now shipped **twice**, and there is still no unit test asserting the behaviour.

The current predicate ([`manage-metadata.ts:5481`](https://github.com/MemberJunction/MJ/blob/next/packages/CodeGenLib/src/Database/manage-metadata.ts#L5481)):

```ts
const emitValidators = skipDBUpdate || ag.featureEnabled('ParseCheckConstraints');
```

That single line is the whole of #4372's fix, and #4370 fixed the other half of the same regression. Neither PR added a test. From #4372's own description:

> Both times that regression shipped, the log said exactly this line and nothing else.

A defect that has regressed twice, silently, with the success log reporting `✔` either way, is the canonical case for a cheap direct assertion.

## Why the drift gate is not sufficient on its own

The `CodeGen drift gate` does catch it end-to-end, which is why #4372 was approvable without this. But as the only guard it has two weaknesses:

1. It needs Docker, SQL Server, 78 migrations and roughly ten minutes, so it cannot run in the fast unit tier.
2. It is the mechanism that *locked in* the loss last time. While the committed artifact had 0 validators, the gate required that output — restoring the 56 overrides failed CI and deleting them passed. A gate that compares against a committed artifact cannot tell you the artifact is the thing that is wrong.

A unit test at the predicate has neither property.

## Suggested shape

With `skipDBUpdate = true` and `ParseCheckConstraints` disabled, a validator must still be queued — and `runValidationGeneration` must be called with `generateNewCode = false`, which is what keeps the LLM out of the load path.

The fixture pattern already exists in the package: `src/__tests__/entity-subclass-codegen.test.ts` and `src/__tests__/idempotency/generated-code-sql.test.ts` both stub `ManageMetadataBase`.

Worth red-proofing against the pre-#4372 predicate, so the test is known to fail without the fix.

## Origin

Found reviewing #4372.

Contributor guide

Open the contributing guide

Research direction

Start at packages/CodeGenLib/src/Database/manage-metadata.ts:5481 and inspect how the emitValidators predicate queues validators. Use the ManageMetadataBase stubbing patterns in src/__tests__/entity-subclass-codegen.test.ts and src/__tests__/idempotency/generated-code-sql.test.ts. Done means a test with skipDBUpdate true and ParseCheckConstraints disabled verifies a validator is queued and runValidationGeneration is called with generateNewCode false.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
testing, tooling
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.