PDX-501: scope comparisonType enum sets by step type#198
Merged
Conversation
RCA: comparisonType is one Provar enum but each step type accepts only a subset; the step-reference, both best-practice rule strings, and docs/mcp.md listed a single flat/incorrect set, so agents emitted load-blocking values such as NotEqualTo on a UI Assert. Fix: scope the guidance into the AssertValues 16-value subset and the UI Assert 6-value subset matching Provar core ComparisonType exactly, in PROVAR_TEST_STEP_REFERENCE.md, both rule strings, and docs/mcp.md, plus schema-aware field-type notes (encrypted->null, rich-text wrapped, Contains direction).
Quality Orchestrator🟢 LOW · No test files mapped. ⚡ quality-orchestrator · |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ProvarDX CLI documentation and best-practice rule messaging to correctly describe comparisonType as step-scoped subsets of com.provar.core.model.base.java.ComparisonType, preventing users from selecting enum values that are load-blocking in the wrong step context (notably NotEqualTo in UI Assert).
Changes:
- Documented the two valid
comparisonTypesubsets separately for UI Assert (uiAttributeAssertion) vs AssertValues (assertValuesComparison). - Updated best-practice rule text to reflect the AssertValues subset and explicitly call out the narrower UI Assert subset.
- Added a step-scoped
comparisonTypetable todocs/mcp.mdand referenced the step-reference resource for full semantics.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/mcp/rules/provar_best_practices_rules.json | Updates rule recommendation/description strings to scope comparisonType guidance to AssertValues and note the UI Assert subset. |
| docs/PROVAR_TEST_STEP_REFERENCE.md | Corrects and expands step reference docs to list step-specific comparisonType subsets and adds field-type/semantics notes for AssertValues. |
| docs/mcp.md | Adds a concise table describing valid comparisonType subsets per step type and explains the load-blocking failure mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
comparisonTypeguidance to present two step-scoped subsets of the singlecom.provar.core.model.base.java.ComparisonTypeenum, instead of one flat/incorrect list. A value used outside its step's subset is load-blocking (IllegalArgumentException: No enum constant …ComparisonType.<value>).assertValuesComparison): 16 values —EqualTo, NotEqualTo, GreaterThan, GreaterThanOrEqualTo, LessThan, LessThanOrEqualTo, IsPresent, IsEmpty, Matches, NotMatches, Contains, NotContains, StartsWith, NotStartsWith, EndsWith, NotEndsWith.uiAttributeAssertion): 6 values —EqualTo, Contains, StartsWith, EndsWith, Matches, None.NotEqualTois valid in AssertValues but not in a UI Assert; the old docs/rules conflated the two contexts.null; rich-text/textarea values arrive wrapped in<p>…</p>so useContains;Containsdirection isexpectedValuecontainsactualValue).Jira
https://provartesting.atlassian.net/browse/PDX-501
Test plan
Changes
docs/PROVAR_TEST_STEP_REFERENCE.md: UI Assert section now lists the 6-value subset with a cross-context explainer; AssertValues section gains the 16-value subset and the comparison/field-type semantics notes.src/mcp/rules/provar_best_practices_rules.json: ASSERT-COMPARISON-001 and ASSERT-VALUES-COMPARISON-001 rule strings scoped to the AssertValues subset, with a note about the narrower UI Assert subset.docs/mcp.md: AssertValues section gains a step-scopedcomparisonTypetable.Note for Provar team
External/customer-facing docs (
docs/provar-mcp-public-docs.md,docs/university-of-provar-mcp-course.md) are maintained separately — if they documentcomparisonType, they should adopt the same two step-scoped sets.