# W1 T2 Review: Onboarding Entities, Repositories, and Review Aggregation

## Verdict

- **Spec compliance:** PASS
- **Task quality:** Approved
- **Overall:** PASS
- **Base:** `7fe96cc`
- **Head:** `ef4a9b8`
- **Blocking findings:** none
- **Cannot verify:** real MySQL 8.4 schema validation and JSON/tag retrieval; H2 `MODE=MySQL` passed.

## Verification

- Read `task-reviewer-prompt.md`, `writing-good-tests.md`, T2 brief, execution contract, design data model, V38, the complete `7fe96cc..ef4a9b8` diff, and the existing `JobReview`, `JobReviewDimensionScore`, `Job`, and `Company` mappings needed to judge the named risks.
- Scope check: the diff contains only the nine files allowed by the T2 brief, with 732 added lines and no unrelated or modified historical files.
- Re-ran the focused T2 evidence command from `mydreamjob-server` with `JAVA_HOME=D:\java` and Maven `3.9.7`.
- Result: `Tests run: 5, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`.
- `git diff --check 7fe96cc..ef4a9b8` reported no whitespace errors.

## Spec Compliance

- `OnboardingChecklistItem.java:27-94` maps the approved table, all fields, unique constraints, defaults, optimistic version, and audit timestamps against `V38__onboarding_checklist_and_company_knowledge.sql:4-30`.
- `request_hash` is mapped with `CHAR(64)` at `OnboardingChecklistItem.java:80`, matching `V38__onboarding_checklist_and_company_knowledge.sql:17`; `OnboardingRepositoryIntegrationTest.java:107-133` verifies both persistence and the migrated H2 column type/length.
- `StudentOnboardingProgress.java:27-63` and `CompanyKnowledge.java:27-62` match the remaining V38 columns, nullability, lengths, unique keys, versions, and audit fields.
- `OnboardingChecklistItemRepository.java:13-25` filters `active=true`, selects active standard rows plus active rows for the requested company, requires both company discriminators for company rows, and orders by `sortOrder ASC, id ASC`.
- `StudentOnboardingProgressRepository.java:14-18` provides both the student/item batch lookup and the exact student/item lookup required by T2.
- `CompanyKnowledgeRepository.java:9-11` provides the unique company lookup.
- `JobReviewRepository.java:35-67` aggregates only reviews whose status is `PUBLISHED` and whose moderation status is `AUTO_APPROVED` or `APPROVED`; all company aggregation is scoped through `jobs.company_id`.
- `JobReviewDimensionScoreRepository.java:46-63` performs the eight-dimension aggregation in one database query and applies the same company, publication-status, and moderation-status filters.
- The review aggregation queries avoid loading review entities or issuing per-job/per-review queries, satisfying the no-N+1 requirement.
- `OnboardingRepositoryIntegrationTest.java:78-195` covers active/inactive standard and company items, cross-company isolation, stable ordering, request-hash mapping, student/item progress scoping, and company knowledge scoping.
- `OnboardingRepositoryIntegrationTest.java:198-264` covers company scoping, moderation filtering, score distribution, raw tags, and dimension aggregation. Tags are selected as the raw database column and ordered deterministically by review ID, as required.
- No service, controller, DTO, state machine, migration, or unrelated production file was added by T2.

## Strengths

- The entity mappings follow the approved V38 schema closely, including the non-obvious `CHAR(64)` request hash.
- The effective-checklist query encodes both company ownership discriminators, preventing a cross-company item from leaking through a mismatched `scope_id` or `company_id`.
- Company review aggregation is implemented as set-based JPQL and consistently applies public-status rules in every query.
- The integration test uses real Spring Data/JPA persistence and independently checks cross-company and moderation boundaries rather than mocking repository behavior.

## Issues

### Critical (Must Fix)

None.

### Important (Should Fix)

None.

### Minor (Nice to Have)

- `OnboardingRepositoryIntegrationTest.java:217-219` labels a review as hidden by setting `moderationStatus=PENDING`, but every company-A review still has `status=PUBLISHED`. A production mutation removing the `r.status = 'PUBLISHED'` predicate from all three company queries would therefore still pass the test. Add a review with `status=HIDDEN` and `moderationStatus=APPROVED`, then assert it is excluded from count, rating distribution, tags, and dimensions. The production queries are currently correct, so this is a coverage gap rather than a runtime defect.
- The checklist query trusts `scope_type=STANDARD` without also enforcing the approved `scope_id=0`/`company_id IS NULL` invariant. The design explicitly assigns that consistency check to the application layer and T2 adds no standard-writing path, so this is only defense in depth.

## Residual Verification Warning

- The implementer correctly disclosed that real MySQL 8.4 was not exercised. H2 `MODE=MySQL` validates the new mappings and executes the aggregation queries successfully, but MySQL schema validation and JSON/tag returns remain a B1 verification-gate check.

## Assessment

**Task quality:** Approved

**Reasoning:** T2 implements the approved persistence mappings and repository queries with correct scoping and public-review filtering, keeps the diff within scope, and passes focused integration evidence. The only findings are non-blocking coverage/defense-in-depth suggestions.

Controller receipt command when `ssf` is available:

```bash
ssf execution review changes/n04-onboard --wave W1 --base 7fe96cc --head ef4a9b8 --report changes/n04-onboard/.superpowers/sdd/reviews/W1-T2.md --verdict pass
```