Files
jobtrackingapp/docs/redesign/phase-9/rd21-remediation/reviews/early-review.md
T

43 lines
8.3 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# RD21 early frontend review
Read-only source review against the Phase 9 frontend at `8e7e3f6387544e24082ebe2c107c0b868897959e`. Product changes are owned by other agents. No tests, servers, network calls or product edits were performed by this reviewer. This report records source findings, not runtime qualification.
## Actionable findings
1. **High — Generic job edit can acknowledge one snapshot and discard later typing.** `job-tracker-ui/src/components/EditJobDialog.tsx:147` submits the full form, but fields at 196–237 and Cancel at 255 remain active while the request is pending. Success at 172–174 closes the dialog regardless of edits made after submission. `save` at 143 has no synchronous pending ref. Retain the original job version, send `expectedVersion`, prevent duplicate submissions, and either freeze fields/close during save or explicitly preserve newer edits. A failure/conflict must keep the dialog draft and show a persistent recoverable error rather than automatically adopting a newer version.
2. **High — Job loading lacks a request identity guard.** `EditJobDialog.tsx:105–139` neither cancels nor rejects an earlier GET when `jobId`/open changes. A delayed response for A can seed the form after B is selected; subsequent PUT uses the current `jobId`. Keep a generation/job identity check on reads and save completion, clear prior loaded identity while loading, and never enable saving a form whose snapshot belongs to another job. `finally` must also belong to the current request.
3. **High — Keep each expected version paired with the draft it qualifies.** `ApplicationAssets.tsx:525–528` deliberately retains a cover-letter draft while `useAsset.reload` replaces `data` at 63. `ApplicationPackageDraftsSection` similarly retains draft across parent refresh at 855–859. When adding concurrency fields, do not simply attach the newest `data.revision`/prop version to an older draft: that would authorize the stale content after a reload. Retain the base version with the first edited snapshot; advance it on successful acknowledgement or explicit discard/reload/review. A conflict must not trigger automatic stale-content resubmission.
4. **Required contract wiring — Career has local serialization but no cross-session token.** `CareerProfilePage.tsx:149` lacks revision in its response type, `loadProfile` at 337–345 does not retain one, PUT at 999 submits the whole structured profile plus raw text, restore at 234 supplies no current version, accept at 414 submits only low-confidence choices, and parse at 906 submits only text. Add response `revision` and request `expectedVersion` to each canonical writer. The reviewed import diff must retain the revision of the comparison the person approved, not silently substitute a subsequently refreshed version. Existing `profileMutationPending` (177, 229, 405, 902, 991) and `editorDisabled` (456) already serialize/block editor mutations locally. Save/restore/review catches retain the draft or choices; preserve this behavior. Success toast is after the mutation promise (1005/240/421), not before persistence acknowledgement.
## Production caller census
| Write surface | API owner | Production caller / entry point |
|---|---|---|
| Generic job PUT | `EditJobDialog.tsx:147` | Shared dialog mounted in `JobTable.tsx:904` and `ApplicationWorkspacePage.tsx:292–296`; workspace header, overview and notes edit actions all open it. No production `updateJob` wrapper was found. |
| Career PUT | `CareerProfilePage.tsx:999` | Explicit Save of canonical fields and raw CV text. Extracted `career/CareerProfileSections.tsx` sections only update parent state. |
| Career restore | `CareerProfilePage.tsx:234` | History restore; dirty editor blocks it at 229. |
| Import accept | `CareerProfilePage.tsx:414` | Reviewed extraction diff and accepted low-confidence IDs. Dirty editor blocks accept at 405/874. Discard at 415 does not write canonical content. |
| Legacy canonical parse | `CareerProfilePage.tsx:906` | Advanced raw-text parse; must use current canonical version and consume returned revision. It also calls `editStructuredCv` and marks the editor dirty at 907. |
| Extraction queue | `CareerProfilePage.tsx:321,709,727,745` | Upload/rebuild/improve/reprocess enqueue extraction; the explicit accept action is the canonical approval boundary. Do not present queued completion as profile-save acknowledgement. |
| Cover-letter save | `applicationWorkspace.ts:298–299` | `ApplicationAssets.tsx:541`, manual/template/accepted AI text through the shared save function. Retain `CoverLetter.revision`; send it as `expectedVersion`. |
| Cover-letter restore | `applicationWorkspace.ts:300–301` | `ApplicationAssets.tsx:571`; existing dirty-discard confirmation at 559–566 and busy guard remain. |
| Answer/recruiter drafts | `applicationWorkspace.ts:310–311` | `ApplicationAssets.tsx:879`; only mounted by `ApplicationWorkspacePage.tsx:264–270`. Parent overview must expose the job version and pass it with the initial content; consume acknowledged `X-Job-Version` for a subsequent save. |
`types.ts:83` (`JobApplication`) and the overview/CoverLetter types in `applicationWorkspace.ts` must expose the backend token. Preserve existing request wrappers' real completion semantics; no success-on-rejection conversion. `ApplicationAssets.tsx:535–551,555–583,873–890` already has synchronous busy refs, disabled fields and catch paths retaining drafts. Its workspace route uses `key={jobId}` at `ApplicationWorkspacePage.tsx:263,265`, so those mutations are remounted on job change; do not expand a generic shared-hook redesign based only on its boolean `isCurrent` helper.
## Related versus separate boundaries
- **Directly related and now runtime-proven by root:** generic job PUT, cover-letter save/restore, answer/recruiter draft replacement all replace user-owned job snapshot fields. One job-version contract must reach all these callers; generic job PUT includes both `notes` (162) and `coverLetterText` (168), so separate editors cannot be treated as independent aggregates without proof.
- **Career compatibility:** legacy `PUT /auth/profile` still accepts `ProfileCvText` and `ProfileCvStructureJson` (`AuthController.cs:535–536`). Current production `ProfilePage.tsx:320` deliberately submits identity fields only. Root/backend review must classify the legacy blob projection path and preserve that identity-only partial-update behavior; this source inspection alone does not prove it overwrites an existing relational canonical aggregate.
- **Dashboard has an established separate revision contract:** `useDashboardLayout.ts:105` sends revision, 108 only applies acknowledged state when request/owner generation is current, 112–113 keeps draft on conflict, and 95 blocks retry of an unresolved conflict. It is a useful existing pattern, not evidence that RD21 affects Dashboard.
- **CV Builder has its own established version contract:** `cvBuilder.ts:278,284,286`; `CvBuilderEditor.tsx:187–205` serializes saves, advances acknowledged version and stops on conflict; current local drafts can be exported at 344. Preserve canonical-versus-CV override and submitted/frozen boundaries. Do not flatten these into the job/Career token.
- **Settings rules are separate:** `SettingsView.tsx:125–147` waits for PUT, retains failed/newer in-flight edits (140–141), and uses a synchronous saving flag. No RD21 cross-session runtime result was established here; record separately rather than broadening this remediation without proof.
- **Documents/timeline:** submitted-package reads/capture in `applicationWorkspace.ts:302–309` are a separate frozen snapshot boundary. Timeline/checklist/interview mutations are item-scoped/additive APIs, not this generic PUT caller. This review does not assert universal concurrency safety for them.
## Regression obligations for the frontend owner
Cover missing/stale expectedVersion without success acknowledgement, retained editable conflict draft/choices, a delayed old-job GET, pending generic-save typing/close behavior, consecutive saves using the acknowledged token, parent refresh while a draft is dirty, and successful navigation/readback after acknowledgement. Preserve existing serialization, tenant/session interceptors (`api.ts:55–106`), canonical/CV override boundaries and frozen exports. Root owns persisted-state/backend and full regression qualification.