PDX-493: feat(mcp) — date/datetime/boolean/integer valueClass dispatch (Thread C, PR 1/2)#183
Open
mrdailey99 wants to merge 1 commit into
Open
PDX-493: feat(mcp) — date/datetime/boolean/integer valueClass dispatch (Thread C, PR 1/2)#183mrdailey99 wants to merge 1 commit into
mrdailey99 wants to merge 1 commit into
Conversation
…a inferSalesforceValueClass helper RCA: Provar runtime silently discards Salesforce date/datetime fields whose XML emits valueClass="string" — the failure surfaces only when a later validation rule reads the empty field. The existing buildArgumentValue dispatch in src/mcp/tools/testCaseGenerate.ts fell through to a hard-coded valueClass="string" fallback after handling variable / compound / uiTarget / uiLocator cases, so every literal date or boolean argument flowed out untyped. A prior helper (inferValueClass) introduced in 3c1545c that covered the boolean case alone was refactored away in 2d4240c, regressing literal-true/false handling too. Datetime, integer, and explicit org-describe-driven type hints were never covered. Fix: Introduce an exported inferSalesforceValueClass(key, val, fieldTypeHint?) helper that returns one of 'date' | 'datetime' | 'boolean' | 'integer' | 'string'. Detection order: explicit fieldTypeHint wins; then ISO-8601 datetime regex /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/; then ISO-8601 date regex /^\d{4}-\d{2}-\d{2}$/; then literal 'true'/'false'; then integer-only string /^-?\d+$/; else string. buildArgumentValue now calls this helper before the (now-removed) string fallback and emits <value class="value" valueClass="<inferred>">…</value>. The fieldTypeHint param is wired into the helper signature but not yet exposed on the MCP tool input schema — that exposure lands in PDX-492 H2b, which sits behind PDX-491 H2a (provar_org_describe). Until then callers omit the hint and detection falls through to the regex layer. Tool description and docs/mcp.md argument-conventions table updated to spell out the auto-detection rules. Tests: New PDX-493 — inferSalesforceValueClass helper block exercises the helper directly across all six branches (datetime with fractional seconds + zone, plain date, true, false, positive integer, negative integer, plain string), the edge case where a short numeric string like "12" must resolve to integer not date (date regex requires full ISO yyyy-mm-dd), strings that look like dates but miss the ISO format must stay string, and explicit fieldTypeHint wins over format detection in all three directions (string-shape + date hint, date-shape + string hint, integer-shape + boolean hint). A second PDX-493 — valueClass emission in generated XML block round-trips through provar_testcase_generate and asserts the inferred attribute reaches the emitted XML for date, datetime, boolean (true and false), positive integer, negative integer, and plain string. Validation: node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts" — 1172 passing / 0 failing; yarn compile clean; yarn lint clean (lint:script-names + lint). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Quality Orchestrator🟢 LOW · 🧪 Tests to Run · Running 1 of 51 tests
▶ Run commandnpx vitest run \
unit/mcp/testCaseGenerate.test.ts⚡ quality-orchestrator · |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds valueClass inference for generated MCP test-case XML so Salesforce-like date, datetime, boolean, and numeric-looking string values are emitted with typed valueClass attributes instead of always falling back to string.
Changes:
- Adds and exports
inferSalesforceValueClass(...)and wires it into argument XML emission. - Adds unit coverage for helper inference and generated XML valueClass output.
- Updates MCP documentation for argument valueClass auto-detection rules.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/mcp/tools/testCaseGenerate.ts |
Adds valueClass inference and updates tool guidance. |
test/unit/mcp/testCaseGenerate.test.ts |
Adds helper and XML emission tests for inferred value classes. |
docs/mcp.md |
Documents the new valueClass inference table and detection order. |
Comments suppressed due to low confidence (3)
src/mcp/tools/testCaseGenerate.ts:406
- This introduces
valueClass="integer", but the repository's canonical step reference documents numeric values asvalueClass="decimal"(for example,docs/PROVAR_TEST_STEP_REFERENCE.md:1337-1338and the data-types checklist at:1428). Please either emit the documented numeric value class here or update the canonical reference with evidence thatintegeris a supported Provar valueClass for these generated arguments.
if (/^-?\d+$/.test(val)) return 'integer';
test/unit/mcp/testCaseGenerate.test.ts:842
- This assertion locks in
valueClass="integer", while the canonical step reference currently documents numeric values asvalueClass="decimal"(docs/PROVAR_TEST_STEP_REFERENCE.md:1337-1338,:1428). Update the expected value class to match the supported Provar numeric type, or add canonical documentation showing thatintegeris valid before testing for it here.
assert.ok(xml.includes('valueClass="integer">42</value>'), `Expected valueClass="integer" for "42"; got: ${xml}`);
test/unit/mcp/testCaseGenerate.test.ts:860
- This also expects
valueClass="integer"for a numeric literal, which conflicts with the canonical step reference'svalueClass="decimal"guidance for numbers (docs/PROVAR_TEST_STEP_REFERENCE.md:1337-1338,:1428). Keep the negative-number case aligned with whichever numeric valueClass is supported.
assert.ok(xml.includes('valueClass="integer">-5</value>'), `Expected valueClass="integer" for "-5"; got: ${xml}`);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fieldTypeHint?: 'date' | 'datetime' | 'boolean' | 'integer' | 'string' | ||
| ): 'date' | 'datetime' | 'boolean' | 'integer' | 'string' { | ||
| if (fieldTypeHint) return fieldTypeHint; | ||
| if (/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/.test(val)) return 'datetime'; |
Comment on lines
+741
to
+744
| | Value matches `^-?\d+$` | `class="value" valueClass="integer"` | Any step | | ||
| | All other values | `class="value" valueClass="string"` | Any step | | ||
|
|
||
| `valueClass` is inferred automatically by `inferSalesforceValueClass(key, val, fieldTypeHint?)`. Detection order: explicit `fieldTypeHint` (wired in a follow-up tool surface — `field_type_hints` param) → ISO-8601 datetime → ISO-8601 date → boolean → integer → string. Provar runtime silently discards date fields emitted as `valueClass="string"`, so always pass date / datetime values in ISO-8601 form. |
Comment on lines
+155
to
+156
| '"true"/"false" → "boolean"; integer-only string (e.g. "42", "-5") → "integer"; otherwise "string". ' + | ||
| 'Pass dates / booleans / integers in those formats — Provar runtime silently discards date fields emitted as valueClass="string".', |
Comment on lines
+724
to
+739
| it('returns "integer" for positive integer string', () => { | ||
| assert.equal(inferSalesforceValueClass('Quantity', '42'), 'integer'); | ||
| }); | ||
|
|
||
| it('returns "integer" for negative integer string', () => { | ||
| assert.equal(inferSalesforceValueClass('Delta', '-5'), 'integer'); | ||
| }); | ||
|
|
||
| it('returns "string" for plain text', () => { | ||
| assert.equal(inferSalesforceValueClass('Name', 'Acme Corp'), 'string'); | ||
| }); | ||
|
|
||
| it('returns "integer" not "date" for a short numeric string like "12"', () => { | ||
| // Edge case: the date regex requires the full ISO yyyy-mm-dd form, so a bare "12" | ||
| // is integer, not date. Guards against false-positive date detection on numeric IDs. | ||
| assert.equal(inferSalesforceValueClass('Code', '12'), 'integer'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
inferSalesforceValueClass(key, val, fieldTypeHint?)helper insrc/mcp/tools/testCaseGenerate.tsand wiresbuildArgumentValueto use it instead of the prior hard-codedvalueClass="string"fallback. The generator now emitsvalueClass="date"/"datetime"/"boolean"/"integer"/"string"based on a strict detection order: explicitfieldTypeHint→ ISO-8601 datetime → ISO-8601 date → literaltrue/false→ integer-only string → string.inferValueClasshelper from 3c1545c) and additionally covers datetime + integer dispatch that was never previously detected. Provar runtime silently discards Salesforce date fields whose XML emitsvalueClass="string"— this fix prevents the silent-drop failure mode.docs/mcp.mdargument-conventions table updated to spell out the auto-detection rules. No new MCP tool surface or input-schema fields are exposed in this PR — thefieldTypeHintparameter is wired into the helper signature only and lands in the MCP input schema in PDX-492 (H2b, Thread C PR 2/2), which sits behind PDX-491 (H2a,provar_org_describe).Scope and threading
This is Thread C, PR 1 of 2 of the PDX-477 plan ("Categories G/H feedback + validate-typo blindspot"). PDX-492 (H2b) — the parameter exposure on
provar_testcase_generate— depends on PDX-491 (H2a,provar_org_describe) landing first and is intentionally out of scope here.No
warningCodes.tsenum entry is added because this PR does not emit any new warning code in PR 1 — the warning emission ("required field missing", "object not in cache") lands in H2b.Files changed
src/mcp/tools/testCaseGenerate.ts— new exported helper, dispatch wired intobuildArgumentValue, tool description updated.test/unit/mcp/testCaseGenerate.test.ts— two new describe blocks (PDX-493 — inferSalesforceValueClass helperandPDX-493 — valueClass emission in generated XML).docs/mcp.md— argument-conventions table extended with the four new value-shape rows + auto-detection paragraph.Test plan
node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts"— 1172 passing / 0 failing (bypasses wireit cache per CLAUDE.md guidance).yarn compile— clean.yarn lint— clean (lint:script-names + lint).yarn build && yarn test) passed.fieldTypeHintoverride directions).provar_testcase_generatefor all six dispatch branches (date, datetime, boolean true, boolean false, positive integer, negative integer, plain string).Not in this PR (flagged for follow-up)
field_type_hintsandrequired_fields_hintparameters onprovar_testcase_generate→ PDX-492 / H2b (Thread C PR 2/2, gated on PDX-491 H2aprovar_org_describe).docs/provar-mcp-public-docs.md,docs/university-of-provar-mcp-course.md) are Provar-team-maintained per CLAUDE.md — flagging here so the Provar team can roll the new valueClass rules into the public docs when they next update.package.json/server.jsonversion bump in this feature PR per the plan's "Coordination chores" rule — version bumps happen in a sweeper PR after each batch of merges.