# Branch: py4lexis4-cmd

**Epic**: Modernize CLI Architecture  
**Goal**: Rework `owilix.cmd` and `owilix.plugins` packages to use Typer consistently  
**Status**: ✅ Cycle 6 - Complete (Logic Migration & Feature Parity)

---

## Cycle 6: Logic Migration & Feature Parity (COMPLETE)

**Completed**: 2026-01-03

### Sub-Cycles Completed:
- ✅ **6a**: Local Commands Parity (`insert`, `free`, `export`) - Extracted to `core/tasks/local.py`
- ✅ **6b**: Metadata Rework (Pydantic Migration) - Moved to `core/models/dataset.py`, Pydantic schema in `core/models/schema.py`
- ✅ **6c**: Query Commands Parity (`slice`, `stream`, `warc`) - Extracted to `core/tasks/query.py`
- ✅ **6d**: Admin & Config & Batch Migration - Added `check`, `batch run` commands
- ✅ **6e**: Logic Extraction & Cleanup - Fixed obsolete imports, updated tests

### Key Changes:
1. **Deleted** `owilix/core/metadata.py` - Moved to `owilix/core/models/dataset.py`
2. **Created** `owilix/core/models/schema.py` - Pydantic DataCite 4.6 models
3. **Renamed** `owilix/core/logic/` → `owilix/core/tasks/`
4. **Created** `owilix/core/tasks/query.py` - Extracted query logic
5. **Created** `owilix/cli/batch.py` - Native batch run command

### Test Status:
- **130 core tests passing**
- 5 tests skipped (external dependencies)
- 1 integration test marked for refactoring

---

## Cycle 7: Cleanup & Stabilization (COMPLETE)

**Completed**: 2026-01-04

### Fixes & Enhancements:
- ✅ **Query Aggregate**: Fixed logic to re-aggregate partial results (count/sum), fixed truncation to show all rows.
- ✅ **Async Query Support**: Added `--async` flag to `aggregate`, `stats`, `sites`. Implemented `query_aggregator_async` wrapper.
- ✅ **Query Slice**: Fixed file discovery bug, correct access level handling, JSON output support.
- ✅ **Documentation**: Detailed docs for all `query` subcommands (`stats`, `slice`, `sites`, `warc`, `aggregate`).
- ✅ **DuckDB Executor**: Fixed protocol handling for split filesystems.

---

## Cycle 5: Full Refactor (COMPLETE)

**Checkpoint Commit**: `4d355ff` (2026-01-03)
**Rollback**: `git checkout 4d355ff` to restore state after Cycle 5

### Goals
1. Remove legacy CLI (`owilix/cli.py`) - Keep only Typer (`owilix/cli/`)
2. Single CLI entry point: `owi` / `owilix` commands
3. Modularize `cmd/base.py` (currently 77KB/1783 lines)
4. Maintain shared function architecture for logging/wrapping
5. Simplify command execution without losing functionality

### Shared Function Architecture (Post-Refactor)

Instead of BaseCommand inheritance, use:
- **`cli/_common/executor.py`**: Command wrapper for timing, logging, error handling
- **`cli/_common/output.py`**: Unified display (OutputWriter)
- **`cmd/base.py`**: Slim base with just execution logic + logging
- **`core/types.py`**: Shared types (CommandResult)

---

## Implementation Progress

### New CLI Package (`owilix/cli/`)

```
owilix/cli/
├── __init__.py         # Main Typer app, global options
├── __main__.py         # Module execution support
├── _common/
│   ├── context.py      # Pydantic CLIContext with OWILIXConsole
│   ├── output.py       # OutputWriter (table/json/jsonl)
│   ├── executor.py     # Command execution wrapper
│   └── progress.py     # Progress bar utilities
├── remote.py           # Remote commands (ls, doctor, pull)
├── local.py            # Local commands (ls, init, insert, free, export)
├── query.py            # Query commands (less, sites, slice, stream, warc, aggregate, analyze)
├── config.py           # Config commands (version, list, get, set)
├── admin.py            # Admin commands (logs, stats, check)
├── batch.py            # Batch commands (run)
└── plugin.py           # Plugin management
```

### Working Commands

| Command | Status | Notes |
|---------|--------|-------|
| `remote ls` | ✅ | Specifier support, table/json output |
| `remote doctor` | ✅ | Shows configured repositories |
| `remote pull` | ✅ | Standard options, delegates to legacy |
| `local ls` | ✅ | List local datasets |
| `local init` | ✅ | Initialize local dataset |
| `local insert` | ✅ | Insert files into dataset |
| `local free` | ✅ | Free dataset space |
| `local export` | ✅ | Export dataset to archive |
| `query less` | ✅ | Interactive browser |
| `query sites` | ✅ | URL filtering |
| `query slice` | ✅ | Dataset slicing |
| `query stream` | ✅ | Data streaming |
| `query warc` | ✅ | WARC extraction |
| `query aggregate` | ✅ | SQL aggregation |
| `query analyze` | ✅ | Transaction log analysis |
| `config list` | ✅ | Show configuration |
| `config get` | ✅ | Get config values |
| `config set` | ✅ | Set config values |
| `admin logs` | ✅ | View logs |
| `admin stats` | ✅ | Usage statistics |
| `admin check` | ✅ | Metadata validation |
| `batch run` | ✅ | Execute batch jobs |

---

## Cycle 1: Analysis and Planning

### Phase 1.1: Current Status Analysis

#### Architecture Overview

```
owilix/
├── cli.py              # Main CLI entry point (Click-based, 505 lines)
├── tcli.py             # Experimental Typer CLI (partial, 310 lines)
└── cmd/                # Command implementations
    ├── __init__.py     # Exports: Local, Remote, Admin, Config, Query, Batch
    ├── base.py         # Base classes (1783 lines!)
    ├── admin.py        # AdminCommands (6.6KB)
    ├── batch.py        # BatchCommands (22KB)
    ├── config.py       # ConfigCommands (7.2KB)
    ├── local.py        # LocalCommands (29KB)
    ├── query.py        # QueryCommands (18.8KB)
    ├── remote.py       # RemoteCommands (32KB)
    ├── workflows.py    # WorkflowCommands (37.8KB)
    └── subcmds/        # Extended subcommands
        ├── query_extended.py   (28.5KB)
        ├── query_graphs.py     (97KB!)
        ├── graph_utils.py      (14KB)
        └── query_warc/         # WARC subcommands

plugins/
    ├── push/           # Push consumers
    │   ├── opensearch.py (16.6KB)
    │   └── convert.py
    ├── ngram/          # N-gram processing
    └── search_consumers.py
```

**Total**: ~320KB of command code, 1783 lines in base.py alone

---

### Current Implementation Issues

#### 1. Dual CLI Framework (Click + Typer)
- **cli.py**: Primary entry point using Click (~505 lines)
- **tcli.py**: Experimental Typer reimplementation (~310 lines, incomplete)
- **Problem**: Maintenance burden, inconsistent behavior

#### 2. Custom SubCommand System (`base.py`)
```python
class SubCommand:
    def __init__(self):
        self.commands = {}
        self.commands["help"] = self.help

    @classmethod
    def register(cls, func):
        # Custom decorator for registering commands

class SubCommandMeta(type):
    # Metaclass for automatic command discovery
```
**Problem**: Non-standard, complex, hard to extend

#### 3. Parameter Parsing Issues
- Uses custom `_cast_args()` for type conversion via pydantic
- String escaping inconsistent (e.g., `files="['**/*']"`)
- Not adhering to CLI conventions like `--flag` vs positional args

#### 4. Giant Base Class Antipattern
`base.py` contains 1783 lines with:
- `BaseCommand` (UI, logging, output formatting)
- `SQLBaseCommands` (extends BaseCommand, 696 lines)
- Helper functions (error collection, progress display)
- Many nested inner functions

#### 5. Command Registration Pattern
```python
@QueryCommands.register
def less(self, local_specifier: str, remote_specifier: str, ...):
```
- Registers functions as methods via decorator
- Inconsistent with Typer's `@app.command()` pattern

---

### Testing Coverage

| Test File | Type | Coverage |
|-----------|------|----------|
| `tests/owilix/cmd/functional_test.py` | Functional | CLI commands via subprocess |
| `tests/owilix/cli/test_smoke.py` | Smoke | Basic CLI invocation |
| `tests/owilix/cli/test_ai_verifiable.py` | AI | Machine-parseable results |

**Missing**: Unit tests for individual command functions

---

### Parameter Parsing Deep Dive

#### Current Flow: CLI → Command

```
CLI Input: owi remote ls all files="['**/*']" format=wide

                    ↓ Click captures

@click.argument('subcmd')                  # "ls"
@click.argument('specifier')               # "all"  
@click.argument('args', nargs=-1)          # ("files=['**/*']", "format=wide")

                    ↓ extract_args()

args = []
kwargs = {"files": "['**/*']", "format": "wide"}

                    ↓ RemoteCommands.do()

                    ↓ _cast_args(func, args, kwargs)

Pydantic TypeAdapter converts types based on function signature
```

#### `extract_args()` in cli.py (Lines 212-238)

```python
def extract_args(args):
    """
    Separates positional and keyword arguments from CLI args.
    - Keyword: contains '=' and NOT wrapped in '{...}'
    - Positional: everything else
    - Escape: wrap in {} to pass literal '=' (e.g., "{a=b}")
    """
    def is_kwargs(x: str):
        return "=" in x and not (x.startswith('{') and x.endswith('}'))
    
    kwargs = {k[0]: k[1] for k in [f.split("=", 1) for f in args if is_kwargs(f)]}
    args = [arg for arg in args if not is_kwargs(arg)]
    return args, kwargs
```

**Issues:**
1. Non-standard: `key=value` instead of `--key value`
2. String escaping awkward: `files="['**/*']"` (needs quotes)
3. `{...}` escape mechanism undocumented
4. `-` to `_` conversion (e.g., `pq-batch-size` → `pq_batch_size`)

#### `_cast_args()` in base.py (Lines 240-307)

```python
def _cast_args(self, func, args, kwargs):
    """
    Uses pydantic TypeAdapter to convert args/kwargs to match 
    function signature types.
    """
    sig = inspect.signature(func)
    for param in sig.parameters.values():
        if param.annotation != inspect.Parameter.empty:
            type_adapter = TypeAdapter(expected_type)
            converted = type_adapter.validate_python(value)
```

**Pros:**
- Automatic type conversion (str → int, str → bool)
- Validation via pydantic

**Cons:**
- Complex error handling
- Hidden behavior (user doesn't see types)

#### Example: Query Less Command

```bash
# Current syntax
owi query less --local . --remote main:latest select=url,title limit=10

# Parsed as:
subcmd = "less"
local = "."
remote = "main:latest"
args = []
kwargs = {"select": "url,title", "limit": "10"}  # ← limit is string!

# _cast_args converts:
kwargs = {"select": "url,title", "limit": 10}  # ← now int
```

---

### Specifier Format (KEEP)

The specifier is a powerful mini-DSL for dataset selection. **✅ Worth keeping and documenting properly.**

#### Syntax

```
<datacenter>:<day>#<duration>/<filter1>=<value1>;<filter2>=<value2>/...
```

#### Components

| Part | Format | Required | Default | Example |
|------|--------|----------|---------|---------|
| `datacenter` | `all`, `lrz`, `it4i`, `csc` | Yes | `all` | `lrz` |
| `day` | `YYYY-MM-DD`, `latest`, `*` | No | - | `2024-03-24`, `latest` |
| `duration` | `#N` (days) | No | - | `#7` (last 7 days) |
| `filters` | `/key=value;key=value` | No | - | `/access=public` |

#### Parsing (manager.py:772-819)

```python
def parse_specifier(self, specifier: str) -> dict:
    """
    Regex: ^(?P<data_center>[\w-]+)(?::(?P<day>[\d-]+|latest)(?:#(?P<duration>\d+))?)?$
    """
    result = {
        'data_center': None,   # "lrz", "it4i", "csc", None for "all"
        'day': None,           # datetime or "latest"
        'duration': None,      # int (days)
        'query': {}            # {"key": "value", ...}
    }
```

#### Examples

| Specifier | Meaning |
|-----------|---------|
| `all` | All datasets from all data centers |
| `lrz:latest` | Latest datasets from LRZ |
| `it4i:2024-03-24` | Datasets from IT4I on specific date |
| `csc:2024-01#7` | CSC datasets, Jan 2024, 7 days |
| `all/access=public` | All public datasets |
| `lrz:latest/collectionName=curlie` | Latest Curlie from LRZ |
| `all/id=f6ea5756-...` | Specific dataset by ID |

#### Design Recommendations for Typer

Keep specifier as a **first positional argument** for brevity:

```bash
# Current (good - keep)
owi remote ls lrz:latest
owi remote pull all:2024-01#7/access=public

# Alternative with Typer (proposed)
owi remote ls lrz:latest          # Argument
owi remote ls --spec lrz:latest   # Option (if needed for clarity)
```

**Exception: Query Commands** use `--local` and `--remote` **options** because:
- Can combine local AND remote in one query
- Both are optional (at least one required)
  
```bash
# Current query syntax (keep)
owi query less --local . --remote main:latest select=url,title
owi query less --remote all/id=f6ea5756-... limit=10

# Proposed Typer
owi query less --local . --remote main:latest --select url,title
```

---

### Typer Migration: Parameter Strategy

#### Proposed Conventions

| Element | Typer | Example |
|---------|-------|---------|
| Required positional | `typer.Argument()` | `specifier: str` |
| Optional with default | `typer.Option()` | `--limit 10` |
| Boolean flags | `typer.Option(is_flag=True)` | `--verbose` |
| Multiple values | `List[str]` | `--file f1 --file f2` |

#### Example: `owi remote ls`

**Current:**
```bash
owi remote ls all format=wide files="['**/*']" no_summary=true
```

**Proposed Typer:**
```bash
owi remote ls all --format wide --files "**/*" --no-summary
owi remote ls lrz:latest -f wide --files "**/*"
```

**Implementation:**
```python
@remote_app.command()
def ls(
    specifier: str = typer.Argument("all", help="Dataset specifier"),
    format: str = typer.Option("short", "--format", "-f", help="Output format"),
    files: Optional[str] = typer.Option(None, help="File glob pattern"),
    no_summary: bool = typer.Option(False, "--no-summary", help="Skip summary"),
    details: bool = typer.Option(False, "--details", "-l", help="Show details")
):
    """List datasets matching SPECIFIER."""
```

#### Balancing Brevity and Extensibility

| Goal | Approach |
|------|----------|
| Short commands | Specifier as positional `Argument` |
| Common options | Short aliases (`-f`, `-l`, `-v`) |
| Extensibility | All other options as `--key value` |
| Backward compat | Support `key=value` via custom parser (optional) |

---

### Documentation

| Location | Content |
|----------|---------|
| `docs/commands.rst` | CLI reference (if exists) |
| `cli.py` docstrings | Main CLI documentation |
| Command class docstrings | Per-command docs |

---

## Proposed Architecture (Typer-based)

### Goals
1. **Single framework**: Typer only (no Click)
2. **Modular**: Each command group in separate file
3. **Extensible**: Plugin system for new commands
4. **Standard**: Consistent parameter handling
5. **Testable**: Commands as pure functions

### Target Structure
```
owilix/
├── cli/                    # New CLI package
│   ├── __init__.py         # Main app
│   ├── main.py             # Entry point, common options
│   ├── remote.py           # remote_app (Typer)
│   ├── local.py            # local_app (Typer)
│   ├── query.py            # query_app (Typer)
│   ├── admin.py            # admin_app (Typer)
│   ├── config.py           # config_app (Typer)
│   ├── batch.py            # batch_app (Typer)
│   └── utils/              # Shared utilities
│       ├── output.py       # Table, JSON, console output
│       ├── context.py      # Shared context (OWIlixManager)
│       └── decorators.py   # Common decorators

cmd/                        # Keep as command logic (no CLI)
    ├── base.py             # SLIM: Just shared logic
    ├── query_logic.py      # Query business logic
    └── ...
```

### Key Changes
1. **Separate CLI from logic**: `owilix/cli/` for CLI, `owilix/cmd/` for business logic
2. **Per-command Typer apps**: Modular, composable
3. **Typer.Option/Argument**: Standard parameter handling
4. **Context dependency injection**: Via Typer callback

---

## Cycle Tasks

### Phase 1.1: Analysis ✅
- [x] Analyze `owilix.cmd` package structure
- [x] Analyze `owilix.plugins` package structure
- [x] Document current implementation issues
- [x] Review test coverage
- [x] Create branch documentation

### Phase 1.2: Planning (TODO)
- [ ] Design target architecture
- [ ] Define migration strategy (incremental vs. big-bang)
- [ ] Identify shared utilities to extract
- [ ] Plan test migration
- [ ] Create implementation plan

### Phase 1.3: Prototype (TODO)
- [ ] Create `owilix/cli/` package skeleton
- [ ] Implement one command end-to-end (e.g., `remote ls`)
- [ ] Verify tests pass
- [ ] Document pattern for other commands

---

## Key Decisions

| Decision | Rationale |
|----------|-----------|
| Keep `owilix.cmd` for logic | Separation of concerns |
| New `owilix.cli` for Typer | Clean slate, no tech debt |
| Incremental migration | Lower risk, testable |
| Keep existing tests | Validate behavior unchanged |

---

## Command Inventory

### LocalCommands
- `ls` - List local datasets
- `remove` - Remove dataset from repository
- `free` - Free local copy
- `insert` - Insert files from filesystem
- `analyze_jsonl` - Analyze JSONL files  
- `export` - Export with CIFF merge

### RemoteCommands
- `ls` - List remote datasets
- `pull` - Download datasets
- `push` - Upload datasets
- `remove` - Remove remote dataset
- `diff` - Compare local/remote
- `doctor` - Check connection status
- `logout` - Logout from remote
- `catalog` - Generate dataset catalog

### QueryCommands
- `less` - Interactive browser
- `sites` - URL-based queries
- `analyze` - Transaction log analysis
- Extended: `slice`, `aggregate`, `stream`, `warc`

### AdminCommands
- (TBD - analyze admin.py)

### ConfigCommands
- (TBD - analyze config.py)

### BatchCommands
- (TBD - analyze batch.py)

---

## References

- [Typer Documentation](https://typer.tiangolo.com/)
- [Click to Typer Migration](https://typer.tiangolo.com/tutorial/commands/one-or-multiple/)
- Current entry point: `owilix/cli.py:main`