MPDX-9888 - hrTools CLAUDE.md#1942
Conversation
Bundle sizes [mpdx-react]Compared against 8b80918 No significant changes found |
zweatshirt
left a comment
There was a problem hiding this comment.
Multi-Agent Code Review — CLEAN
Verdict: CLEAN · Risk: 0/10 (LOW) · 1 file changed (+150 / -0)
Documentation-only change: adds src/components/HrTools/CLAUDE.md, a directory-level guidance file capturing cross-form invariants for the HrTools suite (routing/naming seam, the shared wizard framework, goal-calculator math drift risk, the persistence taxonomy, per-form gotchas, and the mock/prototype tools).
Review approach
Because a CLAUDE.md's only real risk is being inaccurate or rot-prone, this review focused on fact-checking every claim against the codebase (four independent fresh-eyes passes over routing/GraphQL, shared/persistence, goal calculators, and standards/doc-fit) rather than the generic code-oriented agents, which have no surface on a Markdown file.
Outcome
- The bulk of the file was confirmed accurate against code (slug↔GraphQL↔component table, all-primary-API routing, three-independent-calculators structure + gross-up drift risk, PDS specifics, NS scenario split, NSO no-upsert, mock-tools status). Every named file path and symbol resolves.
- Four inaccuracies surfaced during review were corrected before this pass: the MHA gating description (real gate is
mhaEligibility, not acountrycheck), "NsGoalCalculator has no client arithmetic" (softened to "no goal-formula math" since display-level math exists), the wizard framework's consumer count (PanelLayouthas 7 consumers, not 3), and the "two autosave hooks / don't add a third" claim (auseAutosaveCheckboxvariant exists). - Exhaustive "all / none / exactly-two" claims were softened to reduce future rot, since no test ties the prose to the imports.
All corrections verified against code. No blockers, no line-level findings.
Checks
prettier --check: passes- Scope: documentation-only, no code / dependency / schema / API / auth surface
There was a problem hiding this comment.
AI Review Auto-Approval
Risk Level: LOW (0/10)
Verdict: CLEAN (no issues found)
This PR was auto-approved because:
- The multi-agent AI review determined it is low risk
- No blocking issues were found
If you believe this PR needs human review, dismiss this approval and request a review manually.
|
@kegrimes Was hoping to get a quick scan of whether the file details are accurate or not. From reading, I think they all are, but its worth a double check. Also, if there is anything you think I should add to this, please let me know! It's possible we want to describe page access behavior too. |
zweatshirt
left a comment
There was a problem hiding this comment.
🧪 [LAB] Posted by
/quality:agent-review-lab(sandbox) — not the production review, and does NOT trigger auto-approve.
Multi-Agent Code Review — Verdict: CLEAN ✅
Documentation-only PR (src/components/HrTools/CLAUDE.md, +150). Four claim-verification agents audited ~30 specific factual assertions in the doc against the actual code. Nearly all check out — the doc is unusually accurate. No blockers; two low-severity accuracy nits (both severity 4, informational — they do not block a CLEAN verdict).
Risk: LOW (1/10) · single doc file, no executable code.
Findings (Suggestions, severity < 5 — informational)
.CLAUDE/→.claude/(line 8) — git tracks the dir lowercase; uppercase won't resolve on case-sensitive CI.- "Constants are year-versioned (
calculationsYear)" (line 83) —calculationsYearis an NS Formik form field, not a constants-versioning key; client constants take no year param.
Verified accurate (high-signal)
- Structure/routing (9/9): nav group
'hr-tools-page'gated oncanSeeHrTools, the slug↔component↔GraphQL-field table,StaffSavingFundshell, all-primary-API routing (no@rest/@client), theShared/CalculationReports/framework, Adornments,Hcm.graphql, three independent goal calculators. - Financial (8/9):
calculateTotals.ts/ PDScalculations// NS server round-trip, the gross-up+attrition drift duplication, PDS reimbursable floor + Simple-zeroing + HCM 403(b), net-pay budget fallback, rounding-at-write-boundary + hardcoded'USD', NS real-goal vs scenario-goal split. - Persistence & per-form (10/10): both autosave hook families + user lists,
goalSettingsApiMapping.tssingle-drop point, SavingsFundTransfer modal CRUD, NSO Complete-vs-Submit, and all MHA/NSO domain gotchas. Both mock tools confirmed backend-less.
| Agent | Critical | High | Important | Suggestions |
|---|---|---|---|---|
| Structure/Routing | 0 | 0 | 0 | 0 |
| Financial | 0 | 0 | 0 | 1 |
| Persistence/Per-form | 0 | 0 | 0 | 0 |
| Standards/Consistency | 0 | 0 | 0 | 1 |
| Total | 0 | 0 | 0 | 2 |
This is the lab skill: it computes auto-approval eligibility (LOW risk → would be eligible) but never triggers the auto-approve workflow.
|
@zweatshirt I would definitely include the access limitations! I would mention the two layers: But, Senior Staff codes: ASR, Salary Calc, MHA, MPD
New Staff codes: ASR
Part-time Field staff codes:
Paid with designation codes: PDS
Intern codes:
National Expat codes: ASR, Salary Calc, MHA
This will at least ensure that when people make changes, they look out for this because it is super important to ensure users are seeing the correct pages. Also, we might have more us staff groups coming but I am sorting that out with Scott. I'm not sure what the goal calculator and new staff calculator should be exactly but I'm thinking it would be MPD goal calc is for senior staff and new staff is new staff. I don't think this is implemented yet but it would be something we need validate before making those changes. Hope this helps! |
|
There are also certain reports that require a staff account id: ASR, MHA, and salary calc. I'm pretty sure partner reminders does as well, but since we are rolling it out early due to the staff web version breaking, we had to remove that restriction. HCM data doesn't exist in prod yet! |
|
@kegrimes I am curious how much of those decisions are set in stone? We want this file to be as accurate as possible or else Claude will consistently be fed false information that could poison future PRs. I'll run through some of your suggestions and try to add the ones that seem stable and unlikely to change |
|
@kegrimes also, thank you for the info! It's really helpful |
|
@zweatshirt That's a great question! I know adding the hybrid user type was added a couple weeks ago, so things could always be changing. Out of that list, I am not 100% confident about interns. The new us staff groups that Scott wants to add are pretty much how interns are defined now, so we may need to separate international interns or maybe add another piece of data from HCM. But what is for sure definite is "who can see what report". I spent a while tracking down who can see what and I'm very confident ASR, MHA, salary calc, PDS, and MPD (I just realized we do have that implemented) are correct. It is just the matter of if the classification itself changes. |
…s, detailing user type and group eligibility for each tool.
|
@kegrimes Added info about the access limitations. Let me know what you think, thanks Katelyn. I have a change in the agent review skill I'll be fully adding soon that checks for drift in CLAUDE.md files it touches, so hopefully that will help keep this file accurate as we continue dev. |
Description
Adds a single directory-level src/components/HrTools/CLAUDE.md that documents the cross-form "why" for the HrTools suite — the rules and seams you can't see from any one form's code. It auto-loads for anyone (or any agent) working under HrTools/.
What it covers:
Routing/naming seam — no shared HrTools layout, and URL slugs + native GraphQL field names diverge from component names (table included).
Shared wizard framework (Shared/CalculationReports/) — reuse it for step-based forms instead of reinventing the stepper.
Goal-calculator drift risk — the three calculators are independent, their math lives in three places (client/client/server), and the gross-up/attrition logic is duplicated; change one, audit the others.
Persistence taxonomy — field-level autosave vs. single-submit-through-mapping vs. modal CRUD, and which forms use which.
Per-form gotchas — MHA's MHA-only gating + Italian paper carve-out, NSO's no-upsert/OneApp-created record.
Mock/prototype tools — MpdGoalAdmin and MpdSupervisorReport aren't wired to a backend.
Written to stay durable: captures invariants and file pointers, deliberately avoiding formulas, coefficients, and enum lists that would rot. Every claim was fact-checked against the code.
https://jira.cru.org/browse/MPDX-9888
Testing
Checklist:
/quality:agent-reviewcommand locally and fixed any relevant suggestions