PDX-517: refactor(mcp): dedup screen-context rule on structured test_item_id

[mrdailey99]

Summary

UI-SCREEN-CONTEXT-001 (src/mcp/tools/bestPracticesEngine.ts) runs two heuristics — flagAssertsOnNewScreen and flagStepsAfterSave — that can both flag the SAME offending step. To avoid double-penalising, it de-duplicated by parsing the testItemId out of the human-readable violation message (/testItemId=([^\s)]+)/).

That message-parse dedup was fragile:

• Steps with no testItemId render as testItemId=N/A, so two genuinely-different offending steps that both lack a testItemId collided on the shared key N/A and silently merged into one violation (under-count).
• Any rewording of the violation message silently broke the dedup.

This PR replaces the message-parse key with a structured key.

Changes

• Added an optional test_item_id?: string field to the BPViolation interface.
• Added an optional testItemId? param to makeViolation; it sets test_item_id on the returned violation when provided.
• flagAssertsOnNewScreen / flagStepsAfterSave now pass the offending step's stepContext(step).tid so each violation carries its structured test_item_id.
• validateUiAssertScreenContext now de-dups on test_item_id instead of violationTestItemId(message). Absent / N/A / empty ids are treated as always-unique (never merged).
• Removed the now-unused violationTestItemId helper and updated the explanatory comment block to describe the structured-key dedup.
• Tests: added a regression test asserting two distinct testItemId-less offending steps are BOTH reported (no N/A collision); the existing bad-fixture test still pins exactly 1 violation / quality_score 96.25 (same-step double-flag still merges).

No behaviour change for valid input. Severity/weight unchanged (major / 5). No rule added or removed (rule count unchanged); no rule JSON description/notes text changed, so the validation rule registry did not need regeneration. No user-facing docs/mcp.md description changed.

Jira

https://provartesting.atlassian.net/browse/PDX-517

Test plan

• yarn compile — clean (fresh tsc --noEmit clean)
• Full unit suite (nyc mocha "test/**/*.test.ts") — 1508 passing, 0 failing
• New test: two distinct testItemId-less steps both reported (no N/A merge)
• Bad fixture still de-dups to 1 violation / quality_score 96.25 (same step, both heuristics)
• node scripts/mcp-smoke.cjs — 30/30 deterministic tools PASS (3 provar_automation_*/provar_testrun_* env-timeout flakes, unrelated to this change)
• yarn lint — clean (incl. lint:script-names)

[github-actions]

Quality Orchestrator

🟢 LOW · 2 / 100 · All changed files have mapped tests.

---

🧪 Tests to Run · Running 1 of 57 tests

• unit/mcp/bestPracticesEngine.test.ts

▶ Run command

npx vitest run \
  unit/mcp/bestPracticesEngine.test.ts

---

_{⚡ quality-orchestrator  ·  /qo stub <file>  ·  qo analyze-local}