# W1 Wave Review: Onboarding Data Model and Read-Only Query Foundation

## Verdict

- **Wave:** W1
- **Base:** `a2c66b3`
- **Head:** `ef4a9b8`
- **Spec compliance:** PASS
- **Task quality:** PASS
- **Overall:** PASS
- **Blocking findings:** None

## Scope Reviewed

Read and checked:

- `changes/n04-onboard/execution-contract.md`
- `changes/n04-onboard/specs/onboarding-checklist/spec.md`
- `changes/n04-onboard/specs/company-knowledge/spec.md`
- `changes/n04-onboard/design.md`
- `changes/n04-onboard/.superpowers/sdd/task-briefs/T1.md`
- `changes/n04-onboard/.superpowers/sdd/task-briefs/T2.md`
- `changes/n04-onboard/.superpowers/sdd/reviews/W1-T1.md`
- `changes/n04-onboard/.superpowers/sdd/reviews/W1-T2.md`
- Complete `a2c66b3..ef4a9b8` implementation diff, including migrations, entities, repositories, review aggregation queries, and integration tests.

`git diff --check a2c66b3..ef4a9b8` reported no whitespace errors. The wave contains no service, controller, DTO, or state-machine implementation, matching W1 scope.

## Test Evidence

Command run from `mydreamjob-server`:

```powershell
$env:JAVA_HOME='D:\java'; $env:Path='D:\java\bin;'+$env:Path
& 'D:\maven\apache-maven-3.9.7\bin\mvn.cmd' '-Dtest=OnboardingMigrationTest,OnboardingSeedDataIntegrationTest,OnboardingRepositoryIntegrationTest' test
```

Result:

```text
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
```

Per-class result:

- `OnboardingMigrationTest`: 1 test, 0 failures/errors/skips.
- `OnboardingRepositoryIntegrationTest`: 5 tests, 0 failures/errors/skips.
- `OnboardingSeedDataIntegrationTest`: 1 test, 0 failures/errors/skips.

## Cross-Task Integration Review

### Migrations vs Entity Mappings

PASS.

- V38 creates all three approved tables with the contract column set, nullability, lengths, defaults, foreign keys, uniqueness rules, and active/sort index.
- Entity mappings match V38 for all three tables: `OnboardingChecklistItem`, `StudentOnboardingProgress`, and `CompanyKnowledge`.
- `request_hash` is mapped explicitly as `CHAR(64)`, matching V38 and the design requirement.
- Unique constraint annotations on the entities match the database constraint names and column sets:
  - `uk_onboarding_checklist_scope_item`
  - `uk_onboarding_checklist_company_idempotency`
  - `uk_student_onboarding_progress_student_item`
  - `uk_company_knowledge_company`
- The new entities are validated under H2 `MODE=MySQL` with `ddl-auto=validate`; the test scopes validation to the three new onboarding tables because an unrelated legacy H2 type mismatch exists in `auth_tokens`.
- No V1-V37 migration or existing table definition was modified.

### Unique Constraints and Version Fields

PASS.

- Checklist item uniqueness matches the design for standard/company scope and per-company idempotency.
- Student progress is unique on `(student_user_id, item_id)`.
- Company knowledge is unique on `company_id`.
- All three entities use `@Version Long` with builder default `0L`, matching the database `version BIGINT NOT NULL DEFAULT 0`.
- No cross-task mismatch was found between database defaults and entity initial values.

### Effective Checklist Query

PASS for W1.

- `findEffectiveChecklistItems(companyId)` returns only active rows.
- It includes active standard rows and active company rows only when both `scopeId` and `companyId` equal the requested company.
- Ordering is deterministic: `sortOrder ASC, id ASC`.
- Integration coverage verifies inactive standard/company items are excluded, other-company items are excluded, and equal-sort-order rows are stable by ID.
- Non-blocking follow-up: the query trusts `scope_type=STANDARD` without also requiring `scope_id=0` and `company_id IS NULL`. The design assigns that consistency invariant to the application layer, and no W1 write path can create malformed standard rows. W2 should preserve that invariant when adding services.

### Progress Lookup

PASS for W1.

- Batch lookup is scoped by `studentUserId` and a collection of item IDs.
- Exact lookup is scoped by `studentUserId` and `itemId`.
- The database/entity unique constraint guarantees at most one progress row per student/item pair.
- Integration coverage verifies multi-student isolation and correct `completedAt` retrieval.
- Non-blocking follow-up for W2: first-write races must be translated through the unique constraint into the approved idempotent/concurrent behavior; W2 also needs to define behavior for an empty item collection if that path is possible.

### Public-Only Company Review Aggregation

PASS.

- Rating count/average, rating distribution, and tag queries all restrict to `status = 'PUBLISHED'` and moderation status `IN ('AUTO_APPROVED', 'APPROVED')`.
- Dimension aggregation applies the same publication and moderation predicates.
- Every company-level query scopes reviews through `jobs.company_id`; other-company data is excluded.
- Aggregation is set-based and does not introduce an obvious N+1 path.
- Integration coverage verifies company isolation, moderation filtering, rating distribution, tags, and eight-dimension aggregation.
- Non-blocking coverage follow-up: add a review with `status=HIDDEN` and `moderationStatus=APPROVED` to prove the status predicate independently of the moderation predicate. The production queries are currently correct.

### Scope Discipline

PASS.

- Production changes are limited to the three migrations, six onboarding persistence files, and additive methods in the two existing review repositories.
- No existing API behavior, historical migration, or unrelated production file was changed.
- T1 review noted that its commit also added the T2 task brief; this is planning-artifact housekeeping and has no runtime or contract impact.
- No W2/W3 service, controller, DTO, security, or OpenAPI implementation was introduced.

### Readiness for W2

PASS with documented follow-ups.

W2 can build on the W1 interfaces for effective checklist reads, student/item progress lookup, company knowledge lookup, and public company review aggregation. Before W2 closes, it must:

- Validate and enforce standard/company scope consistency in service code.
- Handle progress insertion races and version conflicts according to 9705 semantics.
- Preserve the required PUBLISHED + approved moderation filter in all knowledge-summary paths.
- Parse raw JSON tags robustly across H2 and MySQL, since H2 returns the raw JSON column in a representation that may include outer quoting.
- Preserve deterministic tie-breaking for top tags and ordering of response data.
- Keep the real MySQL 8.4 migration/schema check as the B1 verification-gate obligation; H2 `MODE=MySQL` passed, but real MySQL was not run in this wave.

## Findings

### Critical

None.

### Important

None.

### Minor

- Add the `status=HIDDEN, moderationStatus=APPROVED` aggregation test case noted above.
- Add defense-in-depth scope-consistency enforcement for standard rows in the effective-checklist service path.
- Preserve the existing validation-filter workaround as test-only infrastructure; it should not mask new onboarding mapping mismatches.

## Assessment

W1 is internally consistent and ready to hand off to W2. The migrations, entity mappings, repository queries, and focused integration evidence agree with the approved contract and both specs. No blocking finding prevents W2 from starting.