Skip to content

fix(flow4d): Phase D post-review hardening - #117

Merged
sudoshi merged 1 commit into
mainfrom
fix/flow4d-pathway-bridge-hardening
Jul 28, 2026
Merged

sudoshi merged 1 commit into
mainfrom
fix/flow4d-pathway-bridge-hardening

Conversation

@sudoshi

@sudoshi sudoshi commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Summary

Post-merge hardening of Phase D (#115) after an independent code review. The review found no CRITICAL/HIGH issues and confirmed PHI-opaqueness, dark-serving inertness, and append-only read discipline all hold. These four items are the actionable follow-ups; the feature still ships dark and this PR changes no prod flags.

  1. intdiv(float) in the LOS calc (PatientJourneyService) — Carbon 3's diffInMinutes() returns a float; the raw intdiv(max(0, $diff), 1440) logged an E_DEPRECATED (implicit lossy float→int) on every demo journey-open and would TypeError under PHP 9. Now casts to int first.
  2. isDesignatedDemoEncounter per-request scan (PatientJourneyService) — the ORDER BY admitted_at LIMIT 1 ran on every journey request while the demo flag is on, with no supporting index. Cached 60 s (the designated patient changes at most once per demo refresh).
  3. Minimal columns (PathwayInstanceReadService) — the pathway_instances read now selects only the 3 columns it uses; the table also carries an encrypted source-identity column that should never be pulled into memory.
  4. Duplicate-stable_key guard (ReferenceModelCompiler) — compileVersion() relies on the DB unique constraint, but the public compile() did not; a duplicate key could double an activity. First occurrence now wins.

Documented as limitations (not bugs), unchanged: the narrow client stale-payload window on a just-flipped flag (dark feature only), and stage-level progress (v1 is milestone-level).

Test plan

  • 3 new regression tests: fractional-timestamp LOS with an E_DEPRECATED trap (PHPUnit here isn't configured to fail on deprecations, so the trap is explicit), progressForEncounterId(null) short-circuit, duplicate-stable_key dedup.
  • Full CarePathways + PatientFlow suites: 116 green. Pint clean.
  • No frontend change; no migration; no prod flags touched.

🤖 Generated with Claude Code

Follows the independent review of PR #115 (no CRITICAL/HIGH; PHI-opaqueness,
dark-serving inertness, append-only reads all confirmed).

- PatientJourneyService: cast Carbon 3's float diffInMinutes to int before
  intdiv (was E_DEPRECATED lossy float→int per demo journey-open, TypeError under
  PHP 9); cache the designated-demo-encounter lookup 60s (was an unindexed
  ORDER BY admitted_at scan per journey request while the demo flag is on).
- PathwayInstanceReadService: select only the 3 columns used from
  patient_experience.pathway_instances (the table also carries an encrypted
  source-identity column — never pull it into memory).
- ReferenceModelCompiler: dedup duplicate stable_keys in the public compile()
  (first occurrence wins; compileVersion already relies on the DB unique key).
- Tests: fractional-timestamp LOS regression with an E_DEPRECATED trap (PHPUnit
  here isn't set to fail on deprecations), null-encounter-id short-circuit,
  duplicate-stable_key dedup. 116 CarePathways+PatientFlow green; Pint clean.
@sudoshi
sudoshi merged commit 4e64786 into main Jul 28, 2026
19 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.

1 participant