Skip to content

Tidy: type after param, drop duplicate test, Financial constraint, fire/flood phrasebank - #18

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

Tidy: type after param, drop duplicate test, Financial constraint, fire/flood phrasebank#18
camlambert2000 merged 2 commits into
mainfrom
staging

Conversation

@camlambert2000

Copy link
Copy Markdown
Collaborator

Summary

  • Tighten routeToNextProfileQuestion after? param to a union of the six valid field names (no runtime change; catches bad call sites at compile time).
  • Drop duplicate PNTS-on-children test; keep the PR refactor(profiling): PNTS selections store null not false; routeToNextProfileQuestion skips after-field #17 regression test which covers the same ground with a clearer name.
  • Add Financial entry to needSubcategoryConstraints in serviceMatcher.ts, mirroring the Health and Work pattern — scopes support matches to debt-financial-problems and money-management.
  • Add CRISIS_FIRE_FLOOD_LOCATION and __SUPPORTER phrasebank entries (previously called from crisis.ts but never defined in the phrasebank). Matches adjacent CRISIS_UNDER16_LOCATION pattern: 7 WMCA LAs + "Somewhere else".
  • Backlog update: items 55 and 56 complete; item 59 visible.

Test plan

  • npm test — 96 / 96 pass locally, including the fire/flood council-number tests that exercise the new phrasebank entries and the surviving PNTS-on-children regression test.
  • CI green on staging.

🤖 Generated with Claude Code

camlambert2000 and others added 2 commits April 22, 2026 13:44
…cial subcategory constraint; CRISIS_FIRE_FLOOD_LOCATION phrasebank

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

vercel Bot commented Apr 22, 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 22, 2026 0:45am
streetsupport-platform-va Ready Ready Preview, Comment Apr 22, 2026 0:45am

Request Review

@james-cross james-cross left a comment

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.

Thanks for this.

Tidy PR.

One small follow-up thought: the existing fire/flood tests all start sessions at CRISIS_FIRE_FLOOD_LOCATION, which is why the missing phrasebank entry slipped past CI. A tiny regression test that starts at GATE0, selects 6, and asserts the rendered text doesn't include [Missing phrase: would protect any future safeguarding-adjacent prompt from the same class of bug. Not blocking, more a thought for the backlog.

Comment thread lib/stateMachine.ts
* Returns terminal output when all required fields are collected.
*/
function routeToNextProfileQuestion(session: SessionState, after?: string): RoutingResult {
function routeToNextProfileQuestion(session: SessionState, after?: 'age' | 'gender' | 'lgbtq' | 'convictions' | 'nrpf' | 'children'): 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.

Nice work on this bit, good follow up from the last PR review.

It's worth considering one extra step:
needProfileRequirements just above this is still typed as Record<string, string[]>, so a typo like 'agee' in any of the requirements arrays would silently produce a never-matching branch in here. If we life the union to a shared alias and reuse it, the same protection applies to both.

Not blocking anything, just a small ergonomics win while we're in this file.

Comment thread lib/phrasebank.ts

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 to flag, this is a real bug fix worth calling out. Without these entries, anyone selecting option 6 at GATE0 would have seen [Missing phrase: CRISIS_FIRE_FLOOD_LOCATION] rendered through the phrase() fallback in shared.ts. Mirroring the CRISIS_UNDER16_LOCATION pattern is the right call here.

Nice work.

@camlambert2000
camlambert2000 merged commit 1139be8 into main Apr 28, 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