From 12a9b59255054871ac84297636fc610b3c1166d1 Mon Sep 17 00:00:00 2001 From: Alexandru Marinescu Date: Mon, 30 Mar 2026 11:31:14 +0300 Subject: [PATCH] feat(docs): enhance activity documentation generation and validation scripts --- .../skills/activity-doc-generator/SKILL.md | 17 ++ .../assets/xaml-activity-template.md | 8 +- .../scripts/extract-activity-metadata.py | 126 ++++++++++--- .../scripts/validate-activity-docs.py | 176 ++++++++++++++++++ .gitignore | 1 + 5 files changed, 302 insertions(+), 26 deletions(-) create mode 100644 .claude/skills/activity-doc-generator/scripts/validate-activity-docs.py diff --git a/.claude/skills/activity-doc-generator/SKILL.md b/.claude/skills/activity-doc-generator/SKILL.md index ce6d379c..70f63f50 100644 --- a/.claude/skills/activity-doc-generator/SKILL.md +++ b/.claude/skills/activity-doc-generator/SKILL.md @@ -71,6 +71,22 @@ For each activity in the JSON output, generate a markdown doc using the template 8. **Behavioral context** — When the activity wraps a well-known library or API (e.g., Excel Interop, Orchestrator REST API, SMTP), briefly note this so the AI agent understands the activity's capabilities and limitations. 9. **References** (optional) — If an activity has supplementary reference files (detailed examples, extended guidance, complex scenarios) that would bloat the main doc, place them in a `{ActivityClassName}/` subdirectory alongside the `.md` file and add a References section linking to them. Only include when such files exist. +### Phase 3: Validate Generated Docs (Mandatory) + +Before presenting docs as complete, run the validator script and fail the generation pass if any errors are reported. + +```bash +python {skillPath}/scripts/validate-activity-docs.py "{docsRoot}" --strict +``` + +Validation must fail on: +- broken or missing fenced XML blocks in XAML examples +- output rows incorrectly marked as `Property` instead of `OutArgument`/`InOutArgument` +- mutually exclusive one-of fields both marked `Required: Yes` +- required input properties missing from XAML example attributes +- leaked internal/infrastructure properties (`Body`, `*InputModeSwitch`, `DeprecatedWarning`) +- placeholder empty Input/Output rows (for example, `| `-` | - | - | `-` | - |`) + --- ## Step-by-Step @@ -232,3 +248,4 @@ When documenting: - [ ] Project settings are documented when present (`[ArgumentSettingAttribute]`) - [ ] References section included when supplementary files exist in `{ActivityClassName}/` subdirectory - [ ] `overview.md` lists all activities with correct relative links +- [ ] Validator passes with zero errors (`validate-activity-docs.py --strict`) diff --git a/.claude/skills/activity-doc-generator/assets/xaml-activity-template.md b/.claude/skills/activity-doc-generator/assets/xaml-activity-template.md index fdf3aeb1..8a205147 100644 --- a/.claude/skills/activity-doc-generator/assets/xaml-activity-template.md +++ b/.claude/skills/activity-doc-generator/assets/xaml-activity-template.md @@ -24,6 +24,7 @@ This is the template for generating per-activity markdown documentation files fo ## Properties +{{#if inputProperties}} ### Input | Name | Display Name | Kind | Type | Required | Default | Placeholder | Description | @@ -31,7 +32,9 @@ This is the template for generating per-activity markdown documentation files fo {{#each inputProperties}} | `{{Name}}` | {{DisplayName}} | {{Kind}} | `{{Type}}` | {{Required}} | {{Default}} | {{Placeholder}} | {{Description}} | {{/each}} +{{/if}} +{{#if configProperties}} ### Configuration | Name | Display Name | Type | Default | Description | @@ -39,7 +42,9 @@ This is the template for generating per-activity markdown documentation files fo {{#each configProperties}} | `{{Name}}` | {{DisplayName}} | `{{Type}}` | {{Default}} | {{Description}} | {{/each}} +{{/if}} +{{#if outputProperties}} ### Output | Name | Display Name | Kind | Type | Description | @@ -47,6 +52,7 @@ This is the template for generating per-activity markdown documentation files fo {{#each outputProperties}} | `{{Name}}` | {{DisplayName}} | {{Kind}} | `{{Type}}` | {{Description}} | {{/each}} +{{/if}} {{#if validConfigurations}} ## Valid Configurations @@ -143,7 +149,7 @@ Properties the user provides to configure what the activity does: - For `InArgument`: the `T` type - For plain properties: the property type directly - For enum types: `EnumName` (list values in Enum Reference section) -- **Required**: `Yes` if `[RequiredArgument]` is present or `IsRequired` is set in the ViewModel, otherwise leave empty +- **Required**: `Yes` if `[RequiredArgument]` is present or `IsRequired` is set in the ViewModel, `Conditional` for one-of/mutually-exclusive inputs, otherwise leave empty - **Default**: From `[DefaultValue(x)]`, inline initializer (`= value`), or constructor assignment - **Placeholder**: From the ViewModel's `Placeholder` property (resolved via `.resx`). Shows the expected format to the user (e.g., `"hh:mm:ss"`, `"dd/MM/yyyy"`). Include when present — it helps coding agents provide correctly formatted values and avoids unnecessary errors - **Description**: From the ViewModel's `Tooltip` property, or `[LocalizedDescription]` on the activity class diff --git a/.claude/skills/activity-doc-generator/scripts/extract-activity-metadata.py b/.claude/skills/activity-doc-generator/scripts/extract-activity-metadata.py index 6c6d6973..33a96b3b 100644 --- a/.claude/skills/activity-doc-generator/scripts/extract-activity-metadata.py +++ b/.claude/skills/activity-doc-generator/scripts/extract-activity-metadata.py @@ -59,6 +59,7 @@ import re import sys from pathlib import Path +from typing import Any # Add shared utilities to path (relative to this script's location) sys.path.insert(0, str(Path(__file__).resolve().parent.parent.parent / "shared")) @@ -95,6 +96,12 @@ def parse_metadata_json(json_paths: list[str]) -> tuple[list[dict], list[str]]: category_order: list[str] = [] seen_names: set[str] = set() + def _get_first(data: dict[str, Any], *keys: str, default: Any = None) -> Any: + for key in keys: + if key in data: + return data[key] + return default + for path in json_paths: try: with open(path, "r", encoding="utf-8-sig") as f: @@ -103,33 +110,33 @@ def parse_metadata_json(json_paths: list[str]) -> tuple[list[dict], list[str]]: print(f"Warning: Could not parse metadata file {path}: {e}", file=sys.stderr) continue - if "orderedCategoryDisplayNameKeys" in data: - category_order = data["orderedCategoryDisplayNameKeys"] - - # Support both lowercase "activities" and capitalized "Activities" keys - acts_list = data.get("activities", data.get("Activities", [])) - for act in acts_list: - # Helper to get values from either camelCase or PascalCase keys - def get_value(obj, camel_key, pascal_key=None): - if pascal_key is None: - pascal_key = camel_key[0].upper() + camel_key[1:] - return obj.get(camel_key, obj.get(pascal_key, "")) - - full_name = get_value(act, "fullName", "FullName") + category_order = _get_first( + data, + "orderedCategoryDisplayNameKeys", + "OrderedCategoryDisplayNameKeys", + default=category_order, + ) + + for act in _get_first(data, "activities", "Activities", default=[]): + full_name = _get_first(act, "fullName", "FullName", default="") if not full_name or full_name in seen_names: continue seen_names.add(full_name) activities.append({ "fullName": full_name, - "shortName": get_value(act, "shortName", "ShortName") or full_name.split(".")[-1], - "displayNameKey": get_value(act, "displayNameKey", "DisplayNameKey"), - "descriptionKey": get_value(act, "descriptionKey", "DescriptionKey"), - "categoryKey": get_value(act, "categoryKey", "CategoryKey"), - "viewModelType": get_value(act, "viewModelType", "ViewModelType"), - "codedWorkflowSupport": act.get("codedWorkflowSupport", act.get("CodedWorkflowSupport", False)), - "browsable": act.get("browsable", act.get("Browsable", True)), - "mandatoryParentActivityFullName": get_value(act, "mandatoryParentActivityFullName", "MandatoryParentActivityFullName"), - "properties": act.get("properties", act.get("Properties", [])), + "shortName": _get_first(act, "shortName", "ShortName", default=full_name.split(".")[-1]), + "displayNameKey": _get_first(act, "displayNameKey", "DisplayNameKey"), + "descriptionKey": _get_first(act, "descriptionKey", "DescriptionKey"), + "categoryKey": _get_first(act, "categoryKey", "CategoryKey"), + "viewModelType": _get_first(act, "viewModelType", "ViewModelType"), + "codedWorkflowSupport": _get_first(act, "codedWorkflowSupport", "CodedWorkflowSupport", default=False), + "browsable": _get_first(act, "browsable", "Browsable", default=True), + "mandatoryParentActivityFullName": _get_first( + act, + "mandatoryParentActivityFullName", + "MandatoryParentActivityFullName", + ), + "properties": _get_first(act, "properties", "Properties", default=[]), "metadataFile": path, }) @@ -175,7 +182,17 @@ def find_cs_file_for_class(root: str, full_class_name: str) -> str | None: (?P[A-Za-z_][\w.<>,\[\]\s?]*) )\s+ (?P[A-Za-z_]\w*)\s* - \{\s*get;\s*set;\s*\} + \{\s* + ( + get;\s*(?:private\s+)?set; + | + set;\s*(?:private\s+)?get; + | + get\s*=>[^;]+;\s*set\s*=>[^;]+; + | + set\s*=>[^;]+;\s*get\s*=>[^;]+; + ) + \s*\} (?:\s*=\s*(?P[^;]+))? """, re.VERBOSE | re.MULTILINE | re.DOTALL, @@ -201,13 +218,22 @@ def find_cs_file_for_class(root: str, full_class_name: str) -> str | None: _SKIP_TYPE_PREFIXES = ("ActivityAction", "ActivityFunc") +def _should_skip_property_name(name: str) -> bool: + """Return True for non-user-facing infrastructure property names.""" + if name in _SKIP_PROPERTY_NAMES: + return True + if name == "DeprecatedWarning": + return True + return name.endswith("InputModeSwitch") + + def extract_activity_properties(content: str) -> list[dict]: """Extract public auto-properties from an Activity .cs file.""" properties = [] for m in RE_PROPERTY.finditer(content): name = m.group("name") - if name in _SKIP_PROPERTY_NAMES: + if _should_skip_property_name(name): continue arg_type = m.group("arg_type") @@ -418,12 +444,17 @@ def _merge_single_property( or resolve_key(prop.get("categoryKey"), resx_map) ) + merged_type = prop["type"] + vm_type = vm.get("type") + if merged_type.lower() == "object" and isinstance(vm_type, str) and vm_type.strip(): + merged_type = vm_type.strip() + return { "name": prop["name"], "displayName": display_name, "description": description, "kind": vm.get("kind") or prop["kind"], - "type": prop["type"], + "type": merged_type, "genericType": prop.get("genericType"), "required": vm.get("isRequired") if vm.get("isRequired") is not None else prop["required"], "defaultValue": prop.get("defaultValue"), @@ -472,6 +503,47 @@ def _property_sort_key(prop: dict) -> tuple: return (9999, prop["name"]) +def _normalize_required_flags(merged_props: list[dict]) -> None: + """Normalize requiredness for mutually-exclusive property sets. + + This avoids marking both sides of one-of choices as required. + """ + by_name = {p["name"]: p for p in merged_props} + + one_of_pairs = [ + ("Key", "KeySecureString"), + ("ConnectionString", "ConnectionSecureString"), + ("Password", "SecurePassword"), + ("ProxyPassword", "ProxySecurePassword"), + ("ClientCertificatePassword", "ClientCertificateSecurePassword"), + ("Code", "ScriptFile"), + ("TargetObject", "TargetType"), + ] + + for left, right in one_of_pairs: + if left in by_name and right in by_name and by_name[left].get("required") and by_name[right].get("required"): + by_name[left]["required"] = False + by_name[right]["required"] = False + group = [left, right] + by_name[left]["requiredOneOf"] = group + by_name[right]["requiredOneOf"] = group + + by_group: dict[str, list[dict]] = {} + for prop in merged_props: + group = prop.get("overloadGroup") + if group: + by_group.setdefault(group, []).append(prop) + + for group_name, group_props in by_group.items(): + required_props = [p for p in group_props if p.get("required")] + if len(required_props) > 1: + names = [p["name"] for p in group_props] + for prop in required_props: + prop["required"] = False + prop["requiredOneOf"] = names + prop["requiredGroup"] = group_name + + def merge_activity_data( meta_entry: dict, activity_props: list[dict], @@ -493,9 +565,13 @@ def merge_activity_data( # Add ViewModel-only properties not found in the activity class activity_prop_names = {p["name"] for p in activity_props} for name, vm in vm_metadata.items(): + if _should_skip_property_name(name): + continue if name not in activity_prop_names and not vm.get("notMapped"): merged_props.append(_make_vm_only_property(name, vm, resx_map)) + _normalize_required_flags(merged_props) + merged_props.sort(key=_property_sort_key) return { diff --git a/.claude/skills/activity-doc-generator/scripts/validate-activity-docs.py b/.claude/skills/activity-doc-generator/scripts/validate-activity-docs.py new file mode 100644 index 00000000..0a691227 --- /dev/null +++ b/.claude/skills/activity-doc-generator/scripts/validate-activity-docs.py @@ -0,0 +1,176 @@ +#!/usr/bin/env python3 +"""Validate generated activity markdown docs for common quality regressions. + +Usage: + python validate-activity-docs.py [--strict] +""" + +from __future__ import annotations + +import argparse +import re +import sys +from dataclasses import dataclass +from pathlib import Path + + +@dataclass +class Finding: + path: Path + message: str + + +INTERNAL_NAMES = {"Body", "DeprecatedWarning"} +ONE_OF_PAIRS = [ + ("Key", "KeySecureString"), + ("ConnectionString", "ConnectionSecureString"), + ("Password", "SecurePassword"), + ("ProxyPassword", "ProxySecurePassword"), + ("ClientCertificatePassword", "ClientCertificateSecurePassword"), + ("Code", "ScriptFile"), + ("TargetObject", "TargetType"), +] + + +def _extract_section(text: str, heading: str) -> str | None: + pattern = re.compile( + rf"^###\s+{re.escape(heading)}\s*$\n(?P.*?)(?=^###\s+|^##\s+|\Z)", + re.MULTILINE | re.DOTALL, + ) + match = pattern.search(text) + return match.group("body") if match else None + + +def _parse_markdown_table(section: str) -> list[dict[str, str]]: + rows: list[dict[str, str]] = [] + table_lines = [line for line in section.splitlines() if line.strip().startswith("|")] + if len(table_lines) < 3: + return rows + + headers = [h.strip() for h in table_lines[0].strip("|").split("|")] + for line in table_lines[2:]: + parts = [part.strip() for part in line.strip("|").split("|")] + if len(parts) != len(headers): + continue + row = {headers[i]: parts[i] for i in range(len(headers))} + rows.append(row) + return rows + + +def _unquote_cell(value: str) -> str: + return value.strip().strip("`") + + +def _check_file(path: Path) -> list[Finding]: + findings: list[Finding] = [] + text = path.read_text(encoding="utf-8") + + if not text.endswith("\n"): + findings.append(Finding(path, "Missing trailing newline at end of file")) + + if "`xml" in text: + findings.append(Finding(path, "Found invalid single-backtick XML code fence (`xml)")) + + if text.count("```") % 2 != 0: + findings.append(Finding(path, "Unbalanced markdown fenced code blocks")) + + if "| `-` | - | - | `-` | - |" in text or "| `-` | - | - | - |" in text: + findings.append(Finding(path, "Found placeholder empty table row")) + + if "## XAML Example" in text: + xaml_match = re.search(r"##\s+XAML Example.*?```xml\s*(.*?)\s*```", text, re.DOTALL) + if not xaml_match: + findings.append(Finding(path, "XAML Example section exists but lacks a valid ```xml fenced block")) + xaml_content = "" + else: + xaml_content = xaml_match.group(1) + else: + xaml_content = "" + + input_section = _extract_section(text, "Input") + output_section = _extract_section(text, "Output") + + required_inputs: list[str] = [] + input_rows: list[dict[str, str]] = [] + if input_section: + input_rows = _parse_markdown_table(input_section) + for row in input_rows: + name = _unquote_cell(row.get("Name", "")) + kind = _unquote_cell(row.get("Kind", "")) + required = _unquote_cell(row.get("Required", "")).lower() + if name in INTERNAL_NAMES or name.endswith("InputModeSwitch"): + findings.append(Finding(path, f"Internal property leaked in Input section: {name}")) + if required == "yes": + required_inputs.append(name) + if kind == "Property" and name.endswith("Path") and required == "yes": + # Soft heuristic for likely argument misclassification. + findings.append(Finding(path, f"Likely argument misclassified as Property: {name}")) + + if output_section: + output_rows = _parse_markdown_table(output_section) + for row in output_rows: + name = _unquote_cell(row.get("Name", "")) + kind = _unquote_cell(row.get("Kind", "")) + if name in INTERNAL_NAMES or name.endswith("InputModeSwitch"): + findings.append(Finding(path, f"Internal property leaked in Output section: {name}")) + if kind and kind not in {"OutArgument", "InOutArgument"}: + findings.append(Finding(path, f"Output kind must be OutArgument or InOutArgument, found: {kind}")) + + # One-of required checks + required_set = set(required_inputs) + for left, right in ONE_OF_PAIRS: + if left in required_set and right in required_set: + findings.append(Finding(path, f"Mutually exclusive inputs both marked required: {left}, {right}")) + + # Required fields should appear in XAML example attrs. + if xaml_content: + for name in required_inputs: + if re.search(rf"\b{name}\s*=", xaml_content) is None: + findings.append(Finding(path, f"Required input missing from XAML example: {name}")) + + return findings + + +def _collect_docs(root: Path) -> list[Path]: + return sorted([p for p in root.rglob("*.md") if p.is_file() and p.name.lower() != "readme.md"]) + + +def main() -> int: + parser = argparse.ArgumentParser(description="Validate generated activity markdown docs") + parser.add_argument("docs_root", help="Root folder containing generated docs") + parser.add_argument("--strict", action="store_true", help="Treat findings as blocking failures") + args = parser.parse_args() + + docs_root = Path(args.docs_root).resolve() + if not docs_root.exists(): + print(f"Docs root not found: {docs_root}", file=sys.stderr) + return 2 + + files = _collect_docs(docs_root) + if not files: + print(f"No markdown files found under {docs_root}", file=sys.stderr) + return 2 + + all_findings: list[Finding] = [] + for path in files: + all_findings.extend(_check_file(path)) + + if all_findings: + print("Validation findings:") + for finding in all_findings: + rel = finding.path.relative_to(docs_root) + print(f"- {rel}: {finding.message}") + if args.strict: + print(f"\nValidation failed with {len(all_findings)} finding(s).", file=sys.stderr) + return 1 + + print(f"Validated {len(files)} markdown file(s).") + if all_findings: + print(f"Total findings: {len(all_findings)}") + else: + print("No findings.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.gitignore b/.gitignore index 24c72f18..e95a2a60 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ /.reviews /.agent-status /.worktrees +/.tmp/