lxq/changes/n04-onboard/.superpowers/sdd/reviews/W1-T2.md

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:

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