From a9c16217f56e42a9a10ec34d695efe9d77a56fda Mon Sep 17 00:00:00 2001 From: Alon Yeshurun <98805507+ayeshurun@users.noreply.github.com> Date: Tue, 17 Mar 2026 12:14:50 +0200 Subject: [PATCH 1/4] Add benchmark script for CLI startup performance This script benchmarks the startup performance of the CLI by measuring module import times, CLI invocation times, and heavy dependency loading. It allows comparisons against a baseline branch or tag. --- scripts/benchmark_startup.py | 270 +++++++++++++++++++++++++++++++++++ 1 file changed, 270 insertions(+) create mode 100644 scripts/benchmark_startup.py diff --git a/scripts/benchmark_startup.py b/scripts/benchmark_startup.py new file mode 100644 index 000000000..0d502613c --- /dev/null +++ b/scripts/benchmark_startup.py @@ -0,0 +1,270 @@ +#!/usr/bin/env python3 +""" +Benchmark CLI startup performance. + +Compares the current branch against 'main' (or any other branch) by measuring: + 1. Module import time (fabric_cli.main) + 2. CLI invocation time (fab --version) + 3. Heavy dependency loading (msal, jwt, cryptography, requests, prompt_toolkit) + +Usage: + # Compare current branch against main + python scripts/benchmark_startup.py + + # Compare current branch against a specific branch/tag/commit + python scripts/benchmark_startup.py --baseline v1.3.0 + + # Run only on the current branch (no git checkout) + python scripts/benchmark_startup.py --current-only + + # Change number of iterations (default: 10) + python scripts/benchmark_startup.py --iterations 20 +""" + +import argparse +import importlib +import json +import os +import shutil +import statistics +import subprocess +import sys +import time + + +HEAVY_MODULES = ["msal", "jwt", "cryptography", "requests", "prompt_toolkit", "psutil"] + + +def measure_import_time(iterations: int) -> dict: + """Measure fabric_cli.main import time across multiple iterations.""" + times = [] + for _ in range(iterations): + # Clear all fabric_cli modules from cache + mods = [k for k in sys.modules if k.startswith("fabric_cli")] + for m in mods: + del sys.modules[m] + + start = time.perf_counter() + importlib.import_module("fabric_cli.main") + elapsed_ms = (time.perf_counter() - start) * 1000 + times.append(elapsed_ms) + + return { + "median_ms": round(statistics.median(times), 1), + "min_ms": round(min(times), 1), + "max_ms": round(max(times), 1), + "mean_ms": round(statistics.mean(times), 1), + "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, + "samples": times, + } + + +def check_heavy_modules() -> dict: + """Check which heavy modules are loaded after importing fabric_cli.main.""" + # Clear all fabric_cli modules + mods = [k for k in sys.modules if k.startswith("fabric_cli")] + for m in mods: + del sys.modules[m] + + # Also clear heavy modules + for mod in HEAVY_MODULES: + keys = [k for k in sys.modules if k.startswith(mod)] + for k in keys: + del sys.modules[k] + + importlib.import_module("fabric_cli.main") + + return {mod: mod in sys.modules for mod in HEAVY_MODULES} + + +def measure_cli_time(iterations: int) -> dict: + """Measure 'fab --version' wall-clock time.""" + fab_path = shutil.which("fab") + if not fab_path: + return {"error": "'fab' not found in PATH. Run 'pip install -e .' first."} + + times = [] + for _ in range(iterations): + start = time.perf_counter() + subprocess.run( + [fab_path, "--version"], + capture_output=True, + text=True, + ) + elapsed_ms = (time.perf_counter() - start) * 1000 + times.append(elapsed_ms) + + return { + "median_ms": round(statistics.median(times), 1), + "min_ms": round(min(times), 1), + "max_ms": round(max(times), 1), + "mean_ms": round(statistics.mean(times), 1), + "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, + } + + +def run_benchmark(label: str, iterations: int) -> dict: + """Run all benchmarks and return results.""" + print(f"\n{'=' * 60}") + print(f" Benchmarking: {label}") + print(f"{'=' * 60}") + + # 1. Import time + print(f" Measuring import time ({iterations} iterations)...", end="", flush=True) + import_results = measure_import_time(iterations) + print(f" {import_results['median_ms']:.0f}ms median") + + # 2. Heavy modules + print(" Checking heavy module loading...", end="", flush=True) + heavy_results = check_heavy_modules() + loaded = [m for m, v in heavy_results.items() if v] + print(f" {len(loaded)} loaded: {', '.join(loaded) if loaded else 'none'}") + + # 3. CLI time + print(f" Measuring 'fab --version' ({iterations} iterations)...", end="", flush=True) + cli_results = measure_cli_time(iterations) + if "error" in cli_results: + print(f" {cli_results['error']}") + else: + print(f" {cli_results['median_ms']:.0f}ms median") + + return { + "label": label, + "import_time": import_results, + "heavy_modules": heavy_results, + "cli_time": cli_results, + } + + +def print_comparison(baseline: dict, current: dict): + """Print a formatted comparison table.""" + print(f"\n{'=' * 60}") + print(" COMPARISON") + print(f"{'=' * 60}\n") + + bl = baseline["import_time"]["median_ms"] + cu = current["import_time"]["median_ms"] + diff = bl - cu + pct = (diff / bl * 100) if bl > 0 else 0 + + print(f" {'Metric':<30} {'Baseline':>10} {'Current':>10} {'Change':>10}") + print(f" {'-' * 62}") + print(f" {'Import time (median):':<30} {bl:>9.0f}ms {cu:>9.0f}ms {diff:>+8.0f}ms") + print(f" {'Import improvement:':<30} {'':>10} {'':>10} {pct:>+8.0f}%") + + if "error" not in baseline["cli_time"] and "error" not in current["cli_time"]: + bl_cli = baseline["cli_time"]["median_ms"] + cu_cli = current["cli_time"]["median_ms"] + cli_diff = bl_cli - cu_cli + cli_pct = (cli_diff / bl_cli * 100) if bl_cli > 0 else 0 + print(f" {'CLI time (median):':<30} {bl_cli:>9.0f}ms {cu_cli:>9.0f}ms {cli_diff:>+8.0f}ms") + print(f" {'CLI improvement:':<30} {'':>10} {'':>10} {cli_pct:>+8.0f}%") + + print(f"\n {'Heavy modules at startup:':<30}") + for mod in HEAVY_MODULES: + bl_loaded = "LOADED" if baseline["heavy_modules"].get(mod) else "deferred" + cu_loaded = "LOADED" if current["heavy_modules"].get(mod) else "deferred" + marker = " ✓" if cu_loaded == "deferred" and bl_loaded == "LOADED" else "" + print(f" {mod:<25} {bl_loaded:>10} {cu_loaded:>10}{marker}") + + print() + + +def git_checkout_and_install(ref: str): + """Checkout a git ref and reinstall the package.""" + print(f"\n Switching to '{ref}'...") + subprocess.run(["git", "checkout", ref], capture_output=True, check=True) + subprocess.run( + [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], + capture_output=True, + check=True, + ) + # Clear all cached fabric_cli modules after reinstall + mods = [k for k in sys.modules if k.startswith("fabric_cli")] + for m in mods: + del sys.modules[m] + + +def main(): + parser = argparse.ArgumentParser( + description="Benchmark CLI startup performance between branches." + ) + parser.add_argument( + "--baseline", + default="main", + help="Git ref to compare against (default: main)", + ) + parser.add_argument( + "--iterations", "-n", + type=int, + default=10, + help="Number of iterations per measurement (default: 10)", + ) + parser.add_argument( + "--current-only", + action="store_true", + help="Only benchmark the current branch (skip baseline)", + ) + parser.add_argument( + "--json", + action="store_true", + help="Output results as JSON", + ) + args = parser.parse_args() + + repo_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + os.chdir(repo_root) + + # Get current branch name + result = subprocess.run( + ["git", "rev-parse", "--abbrev-ref", "HEAD"], + capture_output=True, text=True, + ) + current_branch = result.stdout.strip() + + print(f" Repo: {repo_root}") + print(f" Current branch: {current_branch}") + print(f" Baseline: {args.baseline}") + print(f" Iterations: {args.iterations}") + + results = {} + + if not args.current_only: + # Benchmark baseline + try: + git_checkout_and_install(args.baseline) + results["baseline"] = run_benchmark(f"Baseline ({args.baseline})", args.iterations) + except subprocess.CalledProcessError: + print(f"\n ERROR: Could not checkout '{args.baseline}'. Does it exist?") + print(f" Try: python scripts/benchmark_startup.py --current-only") + sys.exit(1) + finally: + # Always return to original branch + subprocess.run(["git", "checkout", current_branch], capture_output=True) + subprocess.run( + [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], + capture_output=True, + ) + # Clear modules again + mods = [k for k in sys.modules if k.startswith("fabric_cli")] + for m in mods: + del sys.modules[m] + + # Benchmark current + results["current"] = run_benchmark(f"Current ({current_branch})", args.iterations) + + # Print comparison + if "baseline" in results: + print_comparison(results["baseline"], results["current"]) + + # JSON output + if args.json: + # Remove raw samples for cleaner JSON + for key in results: + if "samples" in results[key].get("import_time", {}): + del results[key]["import_time"]["samples"] + print(json.dumps(results, indent=2)) + + +if __name__ == "__main__": + main() From 7a563ff783facb8acfd74aa8ca629a88e064dad2 Mon Sep 17 00:00:00 2001 From: Alon Yeshurun <98805507+ayeshurun@users.noreply.github.com> Date: Sun, 22 Mar 2026 12:05:29 +0200 Subject: [PATCH 2/4] Delete scripts/benchmark_startup.py --- scripts/benchmark_startup.py | 270 ----------------------------------- 1 file changed, 270 deletions(-) delete mode 100644 scripts/benchmark_startup.py diff --git a/scripts/benchmark_startup.py b/scripts/benchmark_startup.py deleted file mode 100644 index 0d502613c..000000000 --- a/scripts/benchmark_startup.py +++ /dev/null @@ -1,270 +0,0 @@ -#!/usr/bin/env python3 -""" -Benchmark CLI startup performance. - -Compares the current branch against 'main' (or any other branch) by measuring: - 1. Module import time (fabric_cli.main) - 2. CLI invocation time (fab --version) - 3. Heavy dependency loading (msal, jwt, cryptography, requests, prompt_toolkit) - -Usage: - # Compare current branch against main - python scripts/benchmark_startup.py - - # Compare current branch against a specific branch/tag/commit - python scripts/benchmark_startup.py --baseline v1.3.0 - - # Run only on the current branch (no git checkout) - python scripts/benchmark_startup.py --current-only - - # Change number of iterations (default: 10) - python scripts/benchmark_startup.py --iterations 20 -""" - -import argparse -import importlib -import json -import os -import shutil -import statistics -import subprocess -import sys -import time - - -HEAVY_MODULES = ["msal", "jwt", "cryptography", "requests", "prompt_toolkit", "psutil"] - - -def measure_import_time(iterations: int) -> dict: - """Measure fabric_cli.main import time across multiple iterations.""" - times = [] - for _ in range(iterations): - # Clear all fabric_cli modules from cache - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - start = time.perf_counter() - importlib.import_module("fabric_cli.main") - elapsed_ms = (time.perf_counter() - start) * 1000 - times.append(elapsed_ms) - - return { - "median_ms": round(statistics.median(times), 1), - "min_ms": round(min(times), 1), - "max_ms": round(max(times), 1), - "mean_ms": round(statistics.mean(times), 1), - "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, - "samples": times, - } - - -def check_heavy_modules() -> dict: - """Check which heavy modules are loaded after importing fabric_cli.main.""" - # Clear all fabric_cli modules - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - # Also clear heavy modules - for mod in HEAVY_MODULES: - keys = [k for k in sys.modules if k.startswith(mod)] - for k in keys: - del sys.modules[k] - - importlib.import_module("fabric_cli.main") - - return {mod: mod in sys.modules for mod in HEAVY_MODULES} - - -def measure_cli_time(iterations: int) -> dict: - """Measure 'fab --version' wall-clock time.""" - fab_path = shutil.which("fab") - if not fab_path: - return {"error": "'fab' not found in PATH. Run 'pip install -e .' first."} - - times = [] - for _ in range(iterations): - start = time.perf_counter() - subprocess.run( - [fab_path, "--version"], - capture_output=True, - text=True, - ) - elapsed_ms = (time.perf_counter() - start) * 1000 - times.append(elapsed_ms) - - return { - "median_ms": round(statistics.median(times), 1), - "min_ms": round(min(times), 1), - "max_ms": round(max(times), 1), - "mean_ms": round(statistics.mean(times), 1), - "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, - } - - -def run_benchmark(label: str, iterations: int) -> dict: - """Run all benchmarks and return results.""" - print(f"\n{'=' * 60}") - print(f" Benchmarking: {label}") - print(f"{'=' * 60}") - - # 1. Import time - print(f" Measuring import time ({iterations} iterations)...", end="", flush=True) - import_results = measure_import_time(iterations) - print(f" {import_results['median_ms']:.0f}ms median") - - # 2. Heavy modules - print(" Checking heavy module loading...", end="", flush=True) - heavy_results = check_heavy_modules() - loaded = [m for m, v in heavy_results.items() if v] - print(f" {len(loaded)} loaded: {', '.join(loaded) if loaded else 'none'}") - - # 3. CLI time - print(f" Measuring 'fab --version' ({iterations} iterations)...", end="", flush=True) - cli_results = measure_cli_time(iterations) - if "error" in cli_results: - print(f" {cli_results['error']}") - else: - print(f" {cli_results['median_ms']:.0f}ms median") - - return { - "label": label, - "import_time": import_results, - "heavy_modules": heavy_results, - "cli_time": cli_results, - } - - -def print_comparison(baseline: dict, current: dict): - """Print a formatted comparison table.""" - print(f"\n{'=' * 60}") - print(" COMPARISON") - print(f"{'=' * 60}\n") - - bl = baseline["import_time"]["median_ms"] - cu = current["import_time"]["median_ms"] - diff = bl - cu - pct = (diff / bl * 100) if bl > 0 else 0 - - print(f" {'Metric':<30} {'Baseline':>10} {'Current':>10} {'Change':>10}") - print(f" {'-' * 62}") - print(f" {'Import time (median):':<30} {bl:>9.0f}ms {cu:>9.0f}ms {diff:>+8.0f}ms") - print(f" {'Import improvement:':<30} {'':>10} {'':>10} {pct:>+8.0f}%") - - if "error" not in baseline["cli_time"] and "error" not in current["cli_time"]: - bl_cli = baseline["cli_time"]["median_ms"] - cu_cli = current["cli_time"]["median_ms"] - cli_diff = bl_cli - cu_cli - cli_pct = (cli_diff / bl_cli * 100) if bl_cli > 0 else 0 - print(f" {'CLI time (median):':<30} {bl_cli:>9.0f}ms {cu_cli:>9.0f}ms {cli_diff:>+8.0f}ms") - print(f" {'CLI improvement:':<30} {'':>10} {'':>10} {cli_pct:>+8.0f}%") - - print(f"\n {'Heavy modules at startup:':<30}") - for mod in HEAVY_MODULES: - bl_loaded = "LOADED" if baseline["heavy_modules"].get(mod) else "deferred" - cu_loaded = "LOADED" if current["heavy_modules"].get(mod) else "deferred" - marker = " ✓" if cu_loaded == "deferred" and bl_loaded == "LOADED" else "" - print(f" {mod:<25} {bl_loaded:>10} {cu_loaded:>10}{marker}") - - print() - - -def git_checkout_and_install(ref: str): - """Checkout a git ref and reinstall the package.""" - print(f"\n Switching to '{ref}'...") - subprocess.run(["git", "checkout", ref], capture_output=True, check=True) - subprocess.run( - [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], - capture_output=True, - check=True, - ) - # Clear all cached fabric_cli modules after reinstall - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - -def main(): - parser = argparse.ArgumentParser( - description="Benchmark CLI startup performance between branches." - ) - parser.add_argument( - "--baseline", - default="main", - help="Git ref to compare against (default: main)", - ) - parser.add_argument( - "--iterations", "-n", - type=int, - default=10, - help="Number of iterations per measurement (default: 10)", - ) - parser.add_argument( - "--current-only", - action="store_true", - help="Only benchmark the current branch (skip baseline)", - ) - parser.add_argument( - "--json", - action="store_true", - help="Output results as JSON", - ) - args = parser.parse_args() - - repo_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - os.chdir(repo_root) - - # Get current branch name - result = subprocess.run( - ["git", "rev-parse", "--abbrev-ref", "HEAD"], - capture_output=True, text=True, - ) - current_branch = result.stdout.strip() - - print(f" Repo: {repo_root}") - print(f" Current branch: {current_branch}") - print(f" Baseline: {args.baseline}") - print(f" Iterations: {args.iterations}") - - results = {} - - if not args.current_only: - # Benchmark baseline - try: - git_checkout_and_install(args.baseline) - results["baseline"] = run_benchmark(f"Baseline ({args.baseline})", args.iterations) - except subprocess.CalledProcessError: - print(f"\n ERROR: Could not checkout '{args.baseline}'. Does it exist?") - print(f" Try: python scripts/benchmark_startup.py --current-only") - sys.exit(1) - finally: - # Always return to original branch - subprocess.run(["git", "checkout", current_branch], capture_output=True) - subprocess.run( - [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], - capture_output=True, - ) - # Clear modules again - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - # Benchmark current - results["current"] = run_benchmark(f"Current ({current_branch})", args.iterations) - - # Print comparison - if "baseline" in results: - print_comparison(results["baseline"], results["current"]) - - # JSON output - if args.json: - # Remove raw samples for cleaner JSON - for key in results: - if "samples" in results[key].get("import_time", {}): - del results[key]["import_time"]["samples"] - print(json.dumps(results, indent=2)) - - -if __name__ == "__main__": - main() From 3d4397e85f51a0c588ccda307e7365d3f1def03b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 09:08:00 +0000 Subject: [PATCH 3/4] update new-item-type.agent.md: add command_support.yaml guidance for all item types, prohibit creating VCR cassettes Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com> Agent-Logs-Url: https://github.com/ayeshurun/fabric-cli/sessions/c94f523e-ee18-4151-9b8f-e8cb2e27e41b --- .github/agents/new-item-type.agent.md | 45 ++++++++++++++++++++------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/.github/agents/new-item-type.agent.md b/.github/agents/new-item-type.agent.md index bb5113ecf..308def413 100644 --- a/.github/agents/new-item-type.agent.md +++ b/.github/agents/new-item-type.agent.md @@ -60,7 +60,7 @@ Use this table to determine which steps apply to the new item type before readin ### Simple Item (no definition, no params) -Only needs Steps 1-3, 9 (add to the standard multi-case match), and 11 (ALL_ITEM_TYPES + basic_item_parametrize). +Only needs Steps 1-3, 9 (add to the standard multi-case match), 10 (review `command_support.yaml` for `unsupported_items`), 11 (ALL_ITEM_TYPES + basic_item_parametrize), 12, and 13. **Examples:** `Dashboard`, `Datamart` @@ -72,19 +72,19 @@ Needs Steps 1-4, 9, 10 (export + import + cp + mv), 11 (ALL_ITEM_TYPES + basic_i ### Item with Creation Parameters -Needs Steps 1-3, 7-9, 11 (ALL_ITEM_TYPES but NOT basic_item_parametrize), 12, and 13. +Needs Steps 1-3, 7-9, 10, 11 (ALL_ITEM_TYPES but NOT basic_item_parametrize), 12, and 13. **Examples:** `Lakehouse` (enableSchemas), `Warehouse` (enableCaseInsensitive), `KQLDatabase` (dbType, eventhouseId) ### Item with OneLake Folders -Needs Steps 1-3, 5, 9, 11, 12, and 13. +Needs Steps 1-3, 5, 9, 10, 11, 12, and 13. **Examples:** `Lakehouse` (Files, Tables), `Warehouse` (Files, Tables), `KQLDatabase` (Tables, Shortcut) ### Item with Job Support -Needs Steps 1-3, 6, 9, 11, 12, and 13. +Needs Steps 1-3, 6, 9, 10, 11, 12, and 13. **Examples:** `Notebook` (RunNotebook), `DataPipeline` (Pipeline), `SparkJobDefinition` (sparkjob) @@ -324,10 +324,28 @@ case ItemType.NEW_ITEM: This file controls which CLI commands are enabled for each item type. Add the new item type's snake_case name to the appropriate command sections based on the API Support Matrix from the Prerequisites. -**Always add to these sections** (basic item support): -- No changes needed for `ls`, `cd`, `exists`, `get`, `set`, `rm`, `open`, `mkdir` -- these work for all item types via the generic items API. +> **This step applies to ALL item types, not just items with definition support.** Even simple items may need `unsupported_items` entries, or may need to be added to special command sections. + +#### 10a. Check `unsupported_items` lists (always) + +Review the `unsupported_items` sections for `rm`, `get`, `set`, and `mkdir`. If the new item type does **not** support one of these operations via the Fabric REST API, add it to the relevant `unsupported_items` list. Check existing entries (e.g., `dashboard`, `paginated_report`) for reference. + +> **Ask the requestor:** "Can this item type be deleted (rm), read with detailed properties (get), updated (set), and created (mkdir) via the standard items API?" Add the item to `unsupported_items` for any command it does not support. + +```yaml +# Example: item cannot be deleted or created via API + rm: + unsupported_items: + # ... existing items ... + - new_item # <-- Add here if rm is NOT supported + mkdir: + unsupported_items: + # ... existing items ... + - new_item # <-- Add here if mkdir is NOT supported +``` + +#### 10b. Add to `export` (if the item supports `getItemDefinition` API) -**Add to `export` if the item supports `getItemDefinition` API:** ```yaml commands: fs: @@ -338,7 +356,8 @@ commands: - new_item # <-- Add here, maintain alphabetical order ``` -**Add to `import` if the item supports `createItem` with definition:** +#### 10c. Add to `import` (if the item supports `createItem` with definition) + ```yaml import: supported_items: @@ -346,7 +365,8 @@ commands: - new_item # <-- Add here ``` -**Add to `mv` if the item supports all three definition APIs:** +#### 10d. Add to `mv` (if the item supports all three definition APIs) + ```yaml mv: supported_items: @@ -354,7 +374,8 @@ commands: - new_item # <-- Add here ``` -**Add to `cp` if the item supports all three definition APIs:** +#### 10e. Add to `cp` (if the item supports all three definition APIs) + ```yaml cp: supported_items: @@ -375,6 +396,8 @@ commands: Add the new item type to the parametrized test lists so that existing tests automatically cover the new item type. +> **IMPORTANT: Do NOT create VCR cassette recording files.** Only modify the parametrization lists in `conftest.py`. The recording files under `tests/test_commands/recordings/` are generated separately by running the tests against a live environment and should never be hand-crafted or generated by this agent. + #### 11a. Add to `ALL_ITEM_TYPES` This list drives the comprehensive test suite (cd, ls, exists, rm, get, set, mkdir). @@ -553,7 +576,7 @@ After completing all applicable steps, run these commands to verify each integra | 7. Creation params | `grep -n 'ItemType.NEW_ITEM' src/fabric_cli/utils/fab_cmd_mkdir_utils.py \| grep -i param` | Has creation params | | 8. Creation payload | `grep -n 'ItemType.NEW_ITEM' src/fabric_cli/utils/fab_cmd_mkdir_utils.py \| grep -i payload` | Has creation payload | | 9. Import payload | `grep -n 'ItemType.NEW_ITEM' src/fabric_cli/core/hiearchy/fab_item.py` | Always | -| 10. Command support | `grep -n 'new_item' src/fabric_cli/core/fab_config/command_support.yaml` | Has definitions | +| 10. Command support | `grep -n 'new_item' src/fabric_cli/core/fab_config/command_support.yaml` | Always (check both supported_items and unsupported_items) | | 11a. ALL_ITEM_TYPES | `grep -n 'NEW_ITEM' tests/test_commands/conftest.py \| grep -i all_item` | Always | | 11b. basic_item | `grep -n 'NEW_ITEM' tests/test_commands/conftest.py \| grep -i basic` | Basic items only | | 11c. mv params | `grep -n 'NEW_ITEM' tests/test_commands/conftest.py \| grep -i mv` | Has mv support | From db46e61ddd29b765d6dbeabfb13f823a38e20698 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 09:08:47 +0000 Subject: [PATCH 4/4] address review: clarify Step 10 sub-step references in complexity patterns Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com> Agent-Logs-Url: https://github.com/ayeshurun/fabric-cli/sessions/c94f523e-ee18-4151-9b8f-e8cb2e27e41b --- .github/agents/new-item-type.agent.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/agents/new-item-type.agent.md b/.github/agents/new-item-type.agent.md index 308def413..3a3da6227 100644 --- a/.github/agents/new-item-type.agent.md +++ b/.github/agents/new-item-type.agent.md @@ -60,7 +60,7 @@ Use this table to determine which steps apply to the new item type before readin ### Simple Item (no definition, no params) -Only needs Steps 1-3, 9 (add to the standard multi-case match), 10 (review `command_support.yaml` for `unsupported_items`), 11 (ALL_ITEM_TYPES + basic_item_parametrize), 12, and 13. +Only needs Steps 1-3, 9 (add to the standard multi-case match), 10a (review `command_support.yaml` for `unsupported_items`), 11 (ALL_ITEM_TYPES + basic_item_parametrize), 12, and 13. **Examples:** `Dashboard`, `Datamart` @@ -324,7 +324,7 @@ case ItemType.NEW_ITEM: This file controls which CLI commands are enabled for each item type. Add the new item type's snake_case name to the appropriate command sections based on the API Support Matrix from the Prerequisites. -> **This step applies to ALL item types, not just items with definition support.** Even simple items may need `unsupported_items` entries, or may need to be added to special command sections. +> **This step applies to ALL item types, not just items with definition support.** Sub-step 10a (`unsupported_items` review) always applies. Sub-steps 10b-10e apply only to items with definition support. #### 10a. Check `unsupported_items` lists (always)