Qa engineering requirements skills#129
Conversation
Rosetta Triage ReviewSummary: This PR adds the QA/TG skills layer by introducing a new Findings: [Instruction Quality — Medium]
[Instruction Quality — Medium]
[Architecture — Medium] None of the new modes are wired into any r3 workflow phase (
[Low] Naming of the two new
Positive observations:
Suggestions (optional):
Automated triage by Rosetta agent |
… per github triage
Rosetta Triage ReviewSummary: Adds Findings:
Suggestions:
Automated triage by Rosetta agent |
Rosetta Triage ReviewSummary: This PR carves the QA/TestGen skills layer out of a larger branch into a focused, reviewable PR — adding Instruction-Quality Review (triggered because PR modifies Findings — REQUIRED (must fix before merge): [REQUIRED-1]
[REQUIRED-2]
Findings — SUGGESTIONS: [SUGGESTION-1] [SUGGESTION-2] [SUGGESTION-3] [SUGGESTION-4] Positive highlights:
Overall assessment: PASS WITH SUGGESTIONS — the two REQUIRED items (reference loading style and duplicate Automated triage by Rosetta agent |
isolomatov-gd
left a comment
isolomatov-gd
left a comment
There was a problem hiding this comment.
Many and critical issues
rm yaml-style frontmatter from skill reference file Co-authored-by: Igor Solomatov <isolomatov@griddynamics.com>
isolomatov-gd
left a comment
isolomatov-gd
left a comment
There was a problem hiding this comment.
Majority of your changes must go to testing or your new qa-* skill (actually ALL of them will go to ONE proper skill).
Not requirements, not reverse-engineering, etc.
Rosetta Triage ReviewSummary: This PR adds a comprehensive QA engineering skills layer to Rosetta, introducing synthesis, gap analysis, scenario design (GWT/TestRail), implementation examples, and test execution triage modes as progressive-disclosure references under
Instruction-Quality Assessment: NEEDS WORK — no blocking safety issues; merge after addressing the DRY/boundary findings below. Findings:
Verified safe:
Suggestions:
Automated triage by Rosetta agent |
github-actions
Bot
added
the
documentation
Rosetta Triage ReviewSummary: This PR centralizes QA-engineering modes into the
Instruction-Quality Findings🔴 MUST — Progressive-disclosure violation (misplaced canonical lists)File: The 5-item canonical-list guidance ( Suggestion: Move the block back into 🔴 MUST — Skill names its calling workflow (sibling-awareness violation)File:
A skill must not reference the workflow or subagent that invokes it. Beyond the boundary violation, no step literally named "Tests step" was found in Suggestion: Replace with workflow-agnostic language: "a plain unit-test request with no QA-flow artifacts". 🔴 MUST — Cross-skill deep-linking into
|
Rosetta Triage ReviewSummary: This PR centralizes QA-engineering modes (synthesis, gap analysis, GWT scenario design, TestRail export, code analysis, test execution triage) into Findings:
Suggestions:
Automated triage by Rosetta agent |
isolomatov-gd
left a comment
isolomatov-gd
left a comment
There was a problem hiding this comment.
Please fix testing/SKILL.md
…y and frontmatters from references
Rosetta Triage ReviewSummary: This PR activates the Instruction Quality Review (changes touch MUST FIX:
SHOULD FIX:
NICE TO HAVE:
Positive observations:
Code quality: No obvious bugs or unsafe patterns beyond the findings above. Test coverage: N/A (instruction files). Documentation: PR body clearly explains the consolidation rationale and deferred workflow repointing. Automated triage by Rosetta agent |
📋 Prompt Quality Validation Report❌ Validation FailedThe full markdown report and raw JSON output are available in the workflow artifacts for 5 days. Files With Issues
📄
|
| Severity | Gate | Details |
|---|---|---|
| Medium | Single Responsibility | Problem: The PR inlines a Rosetta-only <rosetta_canonical_lists> block directly into the always-loaded SKILL.md entry file, which mildly tensions the skill's own stated principle that checklists live in references/* to keep the entry file small.Reason: Minor: a Rosetta-specific block is now read on every invocation including non-Rosetta targets, though the guard makes the skip cheap. Solution: Optional: keep the enumerated list in pa-rosetta.md and leave only a short conditional pointer in SKILL.md. Acceptable as-is since the block is 9 lines and explicitly guarded to skip for non-Rosetta targets. |
📄 instructions/r3/core/skills/coding-agents-prompt-authoring/references/pa-rosetta.md
⚠️ Issues Found
| Severity | Gate | Details |
|---|---|---|
| Medium | Reference Integrity | Problem: The canonical-lists block moved out of pa-rosetta.md into the skill's always-loaded SKILL.md entry. The hardening reference pa-hardening.md still says only 'MUST READ SKILL FILE references/pa-rosetta.md and validate prompt uses it' and was not updated, so a late review/hardening pass that loads pa-rosetta.md at point of use no longer finds the canonical-list requirement there. Reason: The canonical-list compliance check may not re-surface at review time; the skill's own principle is that distance and context compaction destroy reliability. Solution: Add a one-line back-reference in pa-rosetta.md's 'Instructions Folder Structure' section pointing to SKILL.md's <rosetta_canonical_lists> block, or update pa-hardening.md's Rosetta check to also re-check that block. |
📄 instructions/r3/core/skills/data-collection/references/documentation-vendor-binding.md
⚠️ Issues Found
| Severity | Gate | Details |
|---|---|---|
| Medium | Conflict Resolution | Problem: The cross-vendor sentence dropped '; the phase owns merging the documentation section with the ticket section' -- the new text ends at 'already present in context', leaving no statement of who merges the documentation output with the ticket output when both bindings run together. Reason: SKILL.md only says cross-vendor aggregation is 'handled externally' in generic terms; without the binding-level restatement naming the two sections, an agent reading only this binding could attempt to merge the documentation section into the ticket section itself, duplicating or conflicting with whatever does own that merge. Solution: Keep a short ownership clause after the search-terms sentence, e.g. '; this binding does not merge sections -- that happens outside this binding', or point explicitly to the SKILL-level aggregation line. |
📄 instructions/r3/core/skills/qa-knowledge/SKILL.md
⚠️ Issues Found
| Severity | Gate | Details |
|---|---|---|
| High | Safety Boundaries | Problem: The frontmatter flip to user-invocable/model-invocable removes the structural guarantee that scenario_design (which can drive a destructive TMS export) and correction (which writes code) only run after the owning workflow's upstream review/approval phases. The 'Standalone (no phase)' branch only tells the agent to ask the user for bindings and paths; it does not re-impose those upstream approvals. Reason: Without it, a user or the model can trigger a TMS export or a code fix from data that never passed the review the multi-phase workflow exists to enforce -- though each destructive mode (testrail-export, correction) still keeps its own inner HITL gate, which limits the blast radius. Solution: In standalone mode, before scenario_design / correction / synthesis proceed, have the agent confirm the upstream artifact (e.g. an approved requirements doc) exists and was reviewed, or state plainly that going standalone waives the phase-level checks. Also reconcile the frontmatter with the skill's README 'Invariants -- do not change' section. |
| High | Rosetta | Problem: qa-knowledge's new <mode_selection> makes the skill behave like a mode-dispatch workflow (7 mutually exclusive deliverables, each with its own validation checklist) rather than the narrower 'reusable knowledge loaded on demand' skill classification; nothing in the diff reconsiders whether this should be a workflow with phases instead. Reason: Keeping prompt types within their defined classification prevents architecture drift where a skill quietly grows into a pipeline coordinator, a role Rosetta reserves for workflows. Solution: Re-evaluate against the Skill vs Workflow classification: if the 7 modes are independent multi-step deliverables, model qa-knowledge (or parts of it) as a workflow with phase files instead of stretching the skill schema. |
| High | Conflict Resolution | Problem: The frontmatter flip (disable-model-invocation: true→false, user-invocable: false→true) directly contradicts the skill's own README.md, whose 'Invariants — do not change' section states: 'disable-model-invocation: true / user-invocable: false must stay: this skill is a routed helper, not a user-facing command.' README.md was not updated in this diff. Reason: Leaving a documented 'do not change' invariant silently broken creates a false record of the skill's contract for maintainers deciding what is safe to edit later. Solution: Update qa-knowledge/README.md's description and invariants section to match the new user-invocable, active-skill status, and fix qa-structure/README.md's 'Like qa-knowledge, it is helper-only' line, which is now also stale. |
| High | Single Responsibility | Problem: The new <mode_selection> packs 7 distinct QA deliverables into one skill -- code_analysis, synthesis, gap_analysis, scenario_design, implementation_modes, test_execution_triage, correction -- each with its own validation-checklist row and router entries. Reason: A skill this wide is harder to review and keep coherent; the healthy range is 1-2 responsibilities, and folding a multi-phase pipeline into one skill blurs the skill-vs-workflow boundary even though the router mitigates runtime cost. Solution: Split into 2-3 narrower skills grouped by natural boundary (e.g. analysis/synthesis vs authoring vs execution/correction), or convert qa-knowledge into a workflow with per-mode phase files, keeping the skill itself to 1-2 responsibilities. |
| Medium | Failure Handling | Problem: The user-invocable flip combines badly with <mode_selection>'s multi-phase rule 'run the earliest, stop; the next phase re-invokes.' In a standalone user invocation there is no next phase to re-invoke, so a user asking for a deliverable spanning several modes gets the earliest mode's output and a dead stop with no continuation guidance. Reason: Otherwise the same change that makes the skill user-invocable leaves a user stranded mid-pipeline, undermining the reliability the mode router is meant to provide. Solution: Add one line to <mode_selection> or core_concepts for the standalone path: after finishing the earliest mode, tell the user which mode to re-invoke next (or that they must re-invoke per mode). |
📄 instructions/r3/core/skills/qa-knowledge/assets/test-spec-template.md
⚠️ Issues Found
| Severity | Gate | Details |
|---|---|---|
| Medium | Structural Coherence | Problem: The intro line still reads '(Outer fence = 4 backticks so the inner 3-backtick example doesn't terminate it.)' but the diff removed the only inner 3-backtick fenced block (the worked ATC example) and replaced it with a plain-text pointer to references/gwt-spec.md. No nested 3-backtick fence remains inside the skeleton. Reason: A stale technical justification pointing at content that no longer exists can mislead a future editor into preserving or reintroducing fence nesting that is no longer necessary. Solution: Delete the '(Outer fence = 4 backticks...)' parenthetical, since it no longer describes anything in the file. |
📄 instructions/r3/core/skills/qa-knowledge/references/gap-analysis-catalogs.md
⚠️ Issues Found
| Severity | Gate | Details |
|---|---|---|
| Medium | Rosetta | Problem: The line pointing to the sibling asset uses '(ACQUIRE FROM KB)' where the rest of the qa-knowledge family uses 'READ SKILL FILE assets/gap-finding-templates.md'. ACQUIRE...FROM KB is a valid Rosetta verb and the target resolves, so this is an alias-consistency slip, not a broken reference. Reason: Minor consistency: mixed alias forms for the same operation add small cognitive load; behavior is unaffected. Solution: For a same-skill sibling asset, prefer 'READ SKILL FILE assets/gap-finding-templates.md' to match the rest of qa-knowledge. |
📄 instructions/r3/core/skills/qa-knowledge/references/synthesis-catalogs.md
⚠️ Issues Found
| Severity | Gate | Details |
|---|---|---|
| Medium | Example Grounding | Problem: All ten output schemas (user-stories, functional/non-functional requirements, constraints, dependencies, assumptions, risks, traceability matrix, out-of-scope, glossary) are shown only as bracket-placeholder templates, e.g. ### US-[N]: [Title], [Specific behavior 1]. No section shows one fully instantiated concrete example, unlike sibling testrail-format.md which supplies three complete worked cases (TC-001, TC-002, TC-003).Reason: Placeholder-only templates are more likely to be filled inconsistently in granularity and tone across runs than templates paired with at least one concrete instance. Solution: Add one small fully-filled example (e.g. one instantiated US-1 / FR-1 / NFR-1 set with real sample values) so the schema is shown in concrete use, not only as an abstract shape. |
📄 instructions/r3/core/skills/qa-knowledge/references/test-execution-triage.md
⚠️ Issues Found
| Severity | Gate | Details |
|---|---|---|
| High | Output Contract | Problem: Step 5 says to 'write findings into the provided findings artifact' but the file never defines that artifact's structure -- no fields, headers, or per-failure record shape appear anywhere. Only the evidence-label values ( Confirmed/Assumption/Unknown) and three worked label examples are given. Compare to sibling testrail-export.md, which spells out an explicit workflow-state record format (exported: <created>/<N> · overlap_count: ... · user_choice: ...).Reason: This is the only qa-knowledge mode with no output template; combined with the now-enabled standalone invocation (no phase to supply the shape), an agent must either stall or improvise the artifact from memory -- the exact behavior the skill's own <anti_patterns> forbids. Solution: Add a findings-artifact skeleton asset (e.g. assets/failure-triage-findings-template.md) with the per-failure fields the validation_checklist already implies (test name, one taxonomy category, one evidence label, cause, cross-failure-pattern flag, priority) and wire it into the SKILL.md router row for test_execution_triage, matching how every sibling mode ships a template. |
📄 instructions/r3/core/skills/qa-knowledge/references/vendor-fork-guide.md
⚠️ Issues Found
| Severity | Gate | Details |
|---|---|---|
| Medium | Workflow Completeness | Problem: The guide never lists the fork procedure as ordered steps (copy file, rename, rebind each table row, run self-validation grep, confirm naming); the same information is spread across un-ordered paragraphs, so the actual sequence to perform a fork must be inferred. Reason: Multi-step procedures should be explicitly ordered so an agent does not skip or resequence steps, e.g. running validation before finishing all rebinds or forgetting the naming step. Solution: Add a short numbered step list near the top (1. copy canonical file, 2. rename per convention, 3. rebind each table row, 4. run self-validation grep, 5. verify naming/discovery) ahead of the supporting detail. |
| Medium | Instruction Ordering | Problem: The 'Discovery (skip this and the fork is orphaned)' rule -- which states the exact file-naming requirement ( <vendor>-format.md / <vendor>-export.md) needed for the SKILL.md router to auto-load the fork -- is the last sentence in the file, appearing after the rationale paragraph and the self-validation grep instructions.Reason: This is the single hard constraint that determines whether the whole fork works at all; hard constraints should precede supporting rationale and validation detail. Solution: Move the Discovery/naming requirement to immediately follow the Rebind table, ahead of the 'Why per-file forks' rationale and the self-validation grep. |
3 participants
Summary
Earlier drafts scattered QA-engineering modes across general-purpose skills and a standalone
scenarios-generationskill — coupling unrelated skills to QA specifics and duplicating conventions. This PR consolidates the whole QA-engineering flow into a single active skill,qa-knowledge, which owns both the active mode routing and the shared reference material it reads on demand.testingstays a general-purpose test-writing skill (untouched here), and no QA modes bleed into general-purpose skills anymore. The standalonescenarios-generationskill is dropped.Key changes
qa-knowledge— the single active QA-engineering skilldisable-model-invocation: false,user-invocable: true); runs the QA flow over the knowledge base it owns and emits into the provided artifact contract (never invents shape or path).<mode_selection>router: pick exactly one of six modes by deliverable, with a no-clean-match fallback (name the closest mode and confirm, never silently pick).code_analysis,synthesis,gap_analysis,scenario_design,implementation_modes, andtest_execution_triage.<dependencies>:reverse-engineeringis a prereq forcode_analysis;coding/debugging/sensitive-dataused at point of use; QA paths & identifiers acquired fromqa-structure.<core_concepts>: lazy point-of-use loading, per-value honesty, total coverage, fail-closed redaction.<validation_checklist>: a per-mode self-check for all six modes.<resources>: a point-of-use router over assets (artifact skeletons) and references (conventions/catalogs), loaded viaREAD SKILL FILE, never all at once.qa-knowledgereferences (9 new)analysis-modes,gap-analysis-catalogs,gwt-spec,implementation-examples,synthesis-catalogs,test-execution-triage,testrail-format,testrail-export,vendor-fork-guide— the shared reference material each mode reads on demand. TMS format/export are vendor bindings (TestRail shipped) with avendor-fork-guidefor retargeting.Also included
coding-agents-prompt-authoring: added a<rosetta_canonical_lists>pointer section (thedocs/definitions/*.mdlists to consult when the target IS Rosetta) and tightened the prompt-classification definitions (skills/subagents are dynamically loaded on demand).data-collectionvendor bindings: removed redundant "base SKILL.md owns the general method" meta-comments (dedup, no behavior change).qa-structure,qa-knowledgeassets: minor convention/path fixes;test-spec-templatenow points togwt-specinstead of restating the ATC format (DRY).docs/definitions/skills.md: droppedscenarios-generation.Note for Github Bot
Cross-skill convention (
READ SKILL FILE/ typed aliases) and the two-location template sync are handled elsewhere. Not touching workflows in this PR — the QA workflows (api-qa/ui-qa/testgenflows) that consume these skills are repointed in a follow-up PR.