Address review: atomic cache, dead code, deprecated filter, tests

- Atomic cache write (temp file + rename) prevents corrupt cache if
  process is killed mid-write
- Skip endpoints marked deprecated:true in OpenAPI spec
- Remove dead snake_case redundancy check
- Add 25 unit tests for pure functions (path naming, prefix detection,
  spec parsing, deprecated flag handling)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: MaorDavidzon

cycode/cli/apps/api/api_command.py

@@ -136,8 +136,8 @@ def build_api_command_groups(
 
         group = click.Group(name=tag_name, help=f'[EXPERIMENT] Cycode API: {tag}')
 
-        # Compute common prefix from all GET endpoint paths in this tag
-        get_endpoints = [ep for ep in endpoints if ep['method'] == 'get']
+        # Compute common prefix from all GET (non-deprecated) endpoint paths in this tag
+        get_endpoints = [ep for ep in endpoints if ep['method'] == 'get' and not ep.get('deprecated')]
         if not get_endpoints:
             continue
 
@@ -152,7 +152,7 @@ def build_api_command_groups(
 
             # Fix redundancy: if command name matches the tag name, use list/view
             # e.g. "cycode groups groups" -> "cycode groups list"
-            if cmd_name == tag_name or cmd_name == tag_name.replace('-', '_'):
+            if cmd_name == tag_name:
                 cmd_name = 'view' if has_path_params else 'list'
 
             # Handle duplicate names (e.g. deprecated + new endpoint for same resource)

cycode/cli/apps/api/openapi_spec.py

@@ -145,10 +145,12 @@ def _fetch_and_cache_spec(client_id: Optional[str] = None, client_secret: Option
 
 
 def _cache_spec(spec: dict) -> None:
-    """Write spec to local cache file."""
+    """Write spec to local cache file atomically (write to temp file, then rename)."""
     try:
         _CACHE_DIR.mkdir(parents=True, exist_ok=True)
-        _CACHE_FILE.write_text(json.dumps(spec), encoding='utf-8')
+        tmp_file = _CACHE_FILE.with_suffix('.json.tmp')
+        tmp_file.write_text(json.dumps(spec), encoding='utf-8')
+        tmp_file.replace(_CACHE_FILE)  # atomic on POSIX and Windows
         logger.debug('Cached OpenAPI spec to %s', _CACHE_FILE)
     except Exception as e:
         logger.warning('Failed to cache OpenAPI spec: %s', e)
@@ -189,6 +191,7 @@ def parse_spec_commands(spec: dict) -> dict[str, list[dict]]:
                 'operation_id': details.get('operationId', ''),
                 'path_params': path_params,
                 'query_params': query_params,
+                'deprecated': details.get('deprecated', False),
             }
 
             if tag not in groups:

tests/cli/apps/api/__init__.py

@@ -0,0 +1,110 @@
+"""Tests for the OpenAPI-to-Click translator."""
+
+from cycode.cli.apps.api.api_command import (
+    _find_common_prefix,
+    _normalize_tag,
+    _param_to_option_name,
+    _path_to_command_name,
+)
+
+# --- _normalize_tag ---
+
+
+def test_normalize_tag_simple() -> None:
+    assert _normalize_tag('Projects') == 'projects'
+
+
+def test_normalize_tag_multi_word() -> None:
+    assert _normalize_tag('Scan Statistics') == 'scan-statistics'
+
+
+def test_normalize_tag_with_special_chars() -> None:
+    assert _normalize_tag('CLI scan statistics') == 'cli-scan-statistics'
+
+
+def test_normalize_tag_strips_leading_trailing_separators() -> None:
+    assert _normalize_tag('  Projects  ') == 'projects'
+
+
+# --- _param_to_option_name ---
+
+
+def test_param_to_option_name_snake_case() -> None:
+    assert _param_to_option_name('page_size') == '--page-size'
+
+
+def test_param_to_option_name_camel_case() -> None:
+    assert _param_to_option_name('pageSize') == '--page-size'
+
+
+def test_param_to_option_name_with_dot() -> None:
+    assert _param_to_option_name('filter.status') == '--filter-status'
+
+
+def test_param_to_option_name_already_kebab() -> None:
+    assert _param_to_option_name('page-size') == '--page-size'
+
+
+# --- _find_common_prefix ---
+
+
+def test_find_common_prefix_empty() -> None:
+    assert _find_common_prefix([]) == ''
+
+
+def test_find_common_prefix_single_path() -> None:
+    # Single path: use parent directory as prefix
+    assert _find_common_prefix(['/v4/projects']) == '/v4'
+
+
+def test_find_common_prefix_two_paths_with_common_parent() -> None:
+    assert _find_common_prefix(['/v4/projects', '/v4/projects/assets']) == '/v4/projects'
+
+
+def test_find_common_prefix_two_paths_with_grandparent() -> None:
+    assert _find_common_prefix(['/v4/projects', '/v4/members']) == '/v4'
+
+
+def test_find_common_prefix_identical_paths() -> None:
+    assert _find_common_prefix(['/v4/projects', '/v4/projects']) == '/v4/projects'
+
+
+# --- _path_to_command_name ---
+
+
+def test_path_to_command_name_collection() -> None:
+    # /v4/projects with prefix /v4/projects -> nothing left -> 'list'
+    assert _path_to_command_name('/v4/projects', '/v4/projects', has_path_params=False) == 'list'
+
+
+def test_path_to_command_name_single_resource() -> None:
+    # /v4/projects/{id} with prefix /v4/projects -> only path param left -> 'view'
+    assert _path_to_command_name('/v4/projects/{projectId}', '/v4/projects', has_path_params=True) == 'view'
+
+
+def test_path_to_command_name_sub_resource() -> None:
+    # /v4/projects/assets with prefix /v4/projects -> 'assets'
+    assert _path_to_command_name('/v4/projects/assets', '/v4/projects', has_path_params=False) == 'assets'
+
+
+def test_path_to_command_name_sub_resource_count() -> None:
+    # /v4/violations/count with prefix /v4/violations -> 'count'
+    assert _path_to_command_name('/v4/violations/count', '/v4/violations', has_path_params=False) == 'count'
+
+
+def test_path_to_command_name_multi_segment() -> None:
+    # /v4/projects/collisions/count with prefix /v4/projects -> 'collisions-count'
+    assert (
+        _path_to_command_name('/v4/projects/collisions/count', '/v4/projects', has_path_params=False)
+        == 'collisions-count'
+    )
+
+
+def test_path_to_command_name_with_path_param_in_middle() -> None:
+    # /v4/workflows/{id}/jobs with prefix /v4/workflows -> 'jobs' (path param stripped)
+    assert _path_to_command_name('/v4/workflows/{workflowId}/jobs', '/v4/workflows', has_path_params=True) == 'jobs'
+
+
+def test_path_to_command_name_kebab_case_normalization() -> None:
+    # Path with underscores or special chars -> kebab-case
+    assert _path_to_command_name('/v4/brokers/broker_metrics', '/v4/brokers', has_path_params=False) == 'broker-metrics'

tests/cli/apps/api/test_api_command.py

@@ -0,0 +1,72 @@
+"""Tests for the OpenAPI spec parser."""
+
+from cycode.cli.apps.api.openapi_spec import parse_spec_commands
+
+
+def test_parse_spec_commands_groups_by_tag() -> None:
+    spec = {
+        'paths': {
+            '/v4/projects': {
+                'get': {'tags': ['Projects'], 'summary': 'Get projects'},
+            },
+            '/v4/violations': {
+                'get': {'tags': ['Violations'], 'summary': 'Get violations'},
+            },
+        }
+    }
+    groups = parse_spec_commands(spec)
+    assert set(groups.keys()) == {'Projects', 'Violations'}
+
+
+def test_parse_spec_commands_extracts_path_params() -> None:
+    spec = {
+        'paths': {
+            '/v4/projects/{projectId}': {
+                'get': {
+                    'tags': ['Projects'],
+                    'parameters': [
+                        {'name': 'projectId', 'in': 'path', 'required': True},
+                        {'name': 'page_size', 'in': 'query', 'required': False},
+                    ],
+                },
+            },
+        }
+    }
+    groups = parse_spec_commands(spec)
+    ep = groups['Projects'][0]
+    assert len(ep['path_params']) == 1
+    assert ep['path_params'][0]['name'] == 'projectId'
+    assert len(ep['query_params']) == 1
+    assert ep['query_params'][0]['name'] == 'page_size'
+
+
+def test_parse_spec_commands_captures_deprecated_flag() -> None:
+    spec = {
+        'paths': {
+            '/v4/old': {
+                'get': {'tags': ['T'], 'summary': 'old', 'deprecated': True},
+            },
+            '/v4/new': {
+                'get': {'tags': ['T'], 'summary': 'new'},
+            },
+        }
+    }
+    groups = parse_spec_commands(spec)
+    by_path = {ep['path']: ep for ep in groups['T']}
+    assert by_path['/v4/old']['deprecated'] is True
+    assert by_path['/v4/new']['deprecated'] is False
+
+
+def test_parse_spec_commands_no_tags_uses_other() -> None:
+    spec = {
+        'paths': {
+            '/v4/foo': {'get': {}},
+        }
+    }
+    groups = parse_spec_commands(spec)
+    assert 'other' in groups
+
+
+def test_parse_spec_commands_empty_spec() -> None:
+    assert parse_spec_commands({}) == {}
+    assert parse_spec_commands({'paths': {}}) == {}