Skip to content

refactor(profiling): PNTS selections store null not false; routeToNextProfileQuestion skips after-field - #17

Merged
camlambert2000 merged 2 commits into
mainfrom
staging
Apr 22, 2026
Merged

refactor(profiling): PNTS selections store null not false; routeToNextProfileQuestion skips after-field#17
camlambert2000 merged 2 commits into
mainfrom
staging

Conversation

@camlambert2000

Copy link
Copy Markdown
Collaborator

Summary

Two coupled changes to fix how "Prefer not to say" selections are stored and routed in the profiling loop.

1. PNTS null storage fix

B5_PROFILE_LGBTQ and B5_PROFILE_CHILDREN handlers now store null (not false) when the user selects "Prefer not to say". This distinguishes PNTS from an explicit "No" answer, matching the housingOptionsInvolvement three-way ternary pattern and the boolean | null type already declared on both fields.

2. after parameter on routeToNextProfileQuestion

Added an optional after parameter that tells the routing loop to skip the named field — preventing null PNTS values from re-triggering the question they just answered.

  • Call sites 1–2 (initial routing from B5_MAIN_SUPPORT_NEED and B5A_ADVICE_TYPE) remain parameter-free
  • Call sites 3–9 (post-handler returns) now pass the field just answered: 'age', 'gender', 'lgbtq', 'convictions', 'nrpf', 'children'

All 9 call sites were audited before building to confirm session.localAuthority is set at each point (guaranteed by the location gate move in PR #16).

Test plan

  • Regression test added: PNTS through B5_PROFILE_CHILDREN does not re-trigger the gate
  • LGBTQ PNTS test updated: asserts lgbtq stores null (was false)
  • 97/97 tests passing

Known issue: stateUpdates propagation (backlog item 59)

Identified during this build: ageCategory, gender, criminalConvictions, and hasChildren handlers return routeToNextProfileQuestion() directly without wrapping stateUpdates, meaning the just-collected field is not propagated back via stateUpdates. lgbtq, lgbtqServicePreference, immigrationStatus, and publicFunds wrap correctly. No routing tests fail because of this, but it is a latent bug — the field is used for in-call routing decisions but lost when route.ts merges stateUpdates back into the session. Logged as item 59 in the backlog for a follow-up PR.

🤖 Generated with Claude Code

camlambert2000 and others added 2 commits April 15, 2026 14:06
…ON on null LA; add find-local-council link to SOMEWHERE_ELSE exit

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…tProfileQuestion skips after-field

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@vercel

vercel Bot commented Apr 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
street-support-va-staging Ready Ready Preview, Comment Apr 15, 2026 1:48pm
streetsupport-platform-va Ready Ready Preview, Comment Apr 15, 2026 1:48pm

Request Review

@camlambert2000 camlambert2000 self-assigned this Apr 15, 2026
expect(result.stateUpdates.currentGate).not.toBe('B5_PROFILE_CHILDREN');
});

test('PNTS through B5_PROFILE_CHILDREN does not re-trigger (regression)', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this new regression test looks like it's covering the same ground as the existing test at line 798 ("User who prefers not to say on children question progresses past the question"). Same session setup, same option selected, same assertion that the state doesn't come back to B5_PROFILE_CHILDREN.

Not a problem for correctness, but two identical tests means if the behaviour ever changes we'll get two failures for the same thing, and one of them will need updating anyway. Could we drop one? Either is fine, though I'd keep the new one if it has a clearer name tied to the fix in this PR.

Comment thread lib/stateMachine.ts
* Returns terminal output when all required fields are collected.
*/
function routeToNextProfileQuestion(session: SessionState): RoutingResult {
function routeToNextProfileQuestion(session: SessionState, after?: string): RoutingResult {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking, but worth considering: typing after as string | undefined means a typo at a call site (e.g. 'chilren') would silently do nothing and quietly re-introduce the bug this PR is fixing. If we constrain it to a union of the actual field names ('age' | 'gender' | 'lgbtq' | 'convictions' | 'nrpf' | 'children'), TypeScript will catch that at compile time.

Small change, and it protects the fix going forward. Up to you.

Comment thread lib/stateMachine.ts

const sessionWithGender = { ...session, gender: profGender };
return routeToNextProfileQuestion(sessionWithGender);
return routeToNextProfileQuestion(sessionWithGender, 'gender');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missing something, but it looks like this handler returns routeToNextProfileQuestion() directly, so the value the user just gave for hasChildren doesn't end up in stateUpdates and gets lost when route.ts merges back into the session.

I can see from the PR description you're already tracking this as backlog item 59. Would mind adding the backlog to tracking and commit it with the next PR? It would be helpful for me to see what's known and what isn't.

@camlambert2000
camlambert2000 merged commit 44eaa3f into main Apr 22, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants