fix: cross-screen coherence found in live walkthrough#36
Conversation
A full walkthrough surfaced eight data/coherence defects where two
screens disagreed or numbers read as synthetic. Fixes:
- Offboarding: a "completed" workflow now actually offboards the
contractor (status + offboarded_at + status history) so the
Contractors list and dashboard no longer show her as Active.
- Classification: overall_score is now the real weighted combination
(IRS 40% + DOL 30% + ABC 30%) of the component scores and the risk
tier is derived from it — the gauge can no longer contradict its own
test scores. Assessments are spread over ~45 days, and the
"Assessed" date uses the app-wide formatter (was M/D/YYYY).
- Invoices: every non-draft invoice gets status-history rows and an
approval step pinned to its real lifecycle timestamps, so a Paid
invoice no longer shows "No status history yet." beside dated
Submitted/Approved/Paid fields.
- Notifications reference real invoice numbers (were INV-2025-* while
all invoices are INV-2026-*) and are spread over the last ~14 days
instead of all showing "27m ago".
- Foreign contractors: required tax form is now W-8BEN (not W-9) on the
contractor detail document status, the contractor's documentStatus
query, the portal documents page, and the portal profile — a foreign
contractor with a current W-8BEN is no longer told it is "missing".
- The contractor linked to the demo contractor login is now John Smith
/ john.smith@example.com, matching the portal greeting and sidebar
(was "James Smith" / james.smith0@example.com).
- Replaced placeholder-style seed strings ("Work item N", "Service
item N", "Project engagement #N") with varied realistic descriptions.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the quality and consistency of seeded data and enhances the handling of tax documents for foreign contractors. Key updates include more realistic descriptions for time entries and invoices, the addition of status history and approval steps for seeded invoices, and UI/API adjustments to support W-8BEN forms for foreign contractors. Feedback highlights a logic issue in the offboarding seed script regarding overlapping status history periods and suggests renaming the hasCurrentW9 field to a more generic name to reflect its updated purpose.
| `UPDATE contractor_status_history SET effective_until = now() | ||
| WHERE contractor_id = $1 AND effective_until IS NULL`, | ||
| [cId], |
There was a problem hiding this comment.
Using now() to close the previous status history entry in a seed script that uses relative dates (via effectiveDate) can lead to overlapping periods in the contractor_status_history table. If effectiveDate is in the past, the old status will be marked as effective until the moment the seed script runs, while the new 'offboarded' status will be marked as effective from the earlier effectiveDate. It is better to use the same timestamp for both closing the old record and starting the new one.
const offboardedAt = new Date(`${effectiveDate}T12:00:00.000Z`).toISOString();
await pool.query(
`UPDATE contractor_status_history SET effective_until = $2
WHERE contractor_id = $1 AND effective_until IS NULL`,
[cId, offboardedAt],
);| // Foreign contractors are required to file a W-8BEN, not a W-9. The | ||
| // documentStatus.hasCurrentW9 field carries "has the current required | ||
| // tax form" regardless of which form that is for this contractor type. | ||
| const requiredTaxForm = contractor.type === 'foreign' ? 'w8ben' : 'w9'; |
There was a problem hiding this comment.
While keeping the hasCurrentW9 field name in the API response avoids breaking changes, it is now semantically misleading as it represents either a W-9 or a W-8BEN depending on the contractor type. Consider refactoring the shared types and the repository to use a more generic name like hasRequiredTaxForm to improve maintainability and clarity for future developers.
1 participant
Why
A full recruiter-style walkthrough of the live site found 8 places where two screens disagreed or numbers read as synthetic. None are crashes — they're exactly the "this looks like demo data, not a real product" tells.
What
completedoffboarding workflow now also sets the contractoroffboarded(+offboarded_at, status-history row).overall_score = round(IRS·0.4 + DOL·0.3 + ABC·0.3),overall_riskderived from canonical thresholds; assessments spread over ~45 days. D2:risk-tab&risk-summary-carduseformatDate.invoice_status_historyrows + anapproval_stepsrow pinned to its real timestamps.invoiceRefsnumbers,created_atspread over ~14 days.contractors.repository.findDetailByIdderives the required tax form from contractor type (W-8BEN for foreign); admin detail label, portal documents (REQUIRED_DOCUMENTS_FOREIGN) and portal profile label made type-aware.hasCurrentW9field name kept (meaning now "has current required tax form").No
Math.random()(CodeQL gate); prior fixes (compliance buckets, audit trail, USD, expiry-aware W-9, generate_series charts) preserved.Verification
Independent local reseed on fresh Postgres + SQL checks: completed→offboarded ✓; 0 weighted-score / band mismatches, 22 distinct assessment days ✓; 0 notifications with dangling invoice refs, 50 distinct past timestamps ✓; 0 non-draft invoices missing status history, 0 reached-approved missing an approval step, 0 future-dated rows ✓; linked contractor = John Smith / john.smith@example.com ✓; 0 foreign W-9 docs / foreign hold W-8BEN ✓.
tsc(api+web) clean,eslint0 errors, API 223 tests + web 34 tests pass. Live before/after screenshots in the final report.