http-latency-prober/docs/plans/2026-07-02-12-05-hxprobe-simplification.md

# hxprobe simplification plan

## Context

The `hxprobe` Python package works and is well-tested, but it carries several
pieces of **speculative scaffolding** — generality added for things that "might
happen later" — plus some **redundant state** and **duplicated logic**. The
user's explicit directive: keep the code simple, with no unnecessary
abstractions or scaffolding for hypothetical future features.

This plan removes that overhead without changing any observable behavior. All
existing tests in `tests/test_cli.py` and `tests/test_probe.py` must continue
to pass unchanged (they are the behavioral contract). Net effect: ~40–50 fewer
lines, fewer moving parts, no new abstractions.

The trigger was a question about the `_Parser._print_message`/`print_help`/
`print_usage` overrides, whose `file=` parameter is accepted but never used —
the classic "kept for a future that never came" smell. Investigation showed the
whole `_Parser` subclass re-implements behavior the standard library already
provides.

---

## Changes

### 1. Delete the `_Parser` subclass — use stdlib stream redirection
**File:** `hxprobe/hxprobe/cli.py` (lines 60–94, and the `run()` wiring ~375–455)

The `_Parser` class + `_ArgExit` exception (~32 lines) exist only to (a) route
argparse's help/usage/error output to the injected `stdout`/`stderr` streams and
(b) raise instead of calling `sys.exit()`. The standard library already does
both:

- `argparse.ArgumentParser` writes help to `sys.stdout` and usage/errors to
  `sys.stderr` by default, and already raises `SystemExit` (not a hard exit) —
  so it is already testable.
- `contextlib.redirect_stdout(stdout)` / `redirect_stderr(stderr)` patch the
  streams argparse writes to.

**Do:**
- Remove `class _ArgExit`, `class _Parser`, and all four overrides
  (`_print_message`, `print_help`, `print_usage`, `error`, `exit`).
- In `run()`, build a plain `argparse.ArgumentParser(prog="hxprobe", ...)` (drop
  the `out=`/`err=` kwargs).
- Wrap the parse + the two manual validations in redirection and catch
  `SystemExit`:

```python
import contextlib
...
try:
    with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr):
        ns = parser.parse_args(args)
        if ns.urls and ns.file:
            parser.error("cannot combine positional url arguments with -f/--file")
        if not ns.urls and not ns.file:
            parser.error("no URLs given (pass as arguments or with -f/--file)")
except SystemExit as exc:
    return EXIT_OK if not exc.code else EXIT_USAGE
```

**Behavior parity (verified against tests):**
- `-h` → argparse prints help to `stdout`, raises `SystemExit(0)` → returns
  `EXIT_OK`. (`test_help_shows_hxprobe_prog_name`, `test_help_lists_protocol_flags`)
- `parser.error(...)` → prints `prog: error: msg` + usage to `stderr`, raises
  `SystemExit(2)` → mapped to `EXIT_USAGE`. (`test_*_is_usage_error`)
- Note: stdlib `error()` exits with code 2; we normalise any non-zero parse exit
  to `EXIT_USAGE` (1) at the single catch site, replacing the per-method code
  baked into the old override.

Only the parse/validate block needs redirection; the rest of `run()` keeps
writing directly to the passed `stdout`/`stderr`.

### 2. Trim `_TimingStream.get_extra_info` to the branch that's actually used
**File:** `hxprobe/hxprobe/probe.py` (lines 262–275)

`server_addr` and `client_addr` are only ever queried by `httpx/_main.py` (the
`httpx` CLI command), never by the request path hxprobe drives — verified by
grepping the installed `httpcore`/`httpx`. The only branch httpcore's sync
connection path calls is `ssl_object` (and `is_readable`, which we intentionally
leave unhandled → `None`).

**Do:** reduce the method to:
```python
def get_extra_info(self, info: str):
    if info == "ssl_object" and isinstance(self._sock, ssl.SSLSocket):
        return self._sock
    return None
```

### 3. Drop the `_load_urls` "future annotations" scaffolding
**File:** `hxprobe/hxprobe/cli.py` (lines 358–369)

The `line.split()[0]` + docstring ("forward-compatible with future
'url  key=value' annotations") is scaffolding for a feature that doesn't exist.
Use the stripped line directly and simplify the docstring to describe what it
actually does (one URL per line, `#` comments and blank lines skipped).
`test_reads_urls_from_file` (comments + blanks) still passes.

### 4. Remove redundant `_set` flags in `_Trace`
**File:** `hxprobe/hxprobe/probe.py` (lines 126–191)

`_dns_set`, `_connect_set`, `_tls_set` duplicate information already carried by
`Phase.present` on the corresponding `self.dns` / `self.connect` / `self.tls`.
The initial `Phase()` has `present=False`, so the first-hop-wins guard is
identical.

**Do:** delete the three boolean fields; replace each guard, e.g.
`if not self._dns_set:` → `if not self.dns.present:` (same for connect/tls).
Keeps the redirect first-hop-wins semantics documented at probe.py:114–123.

### 5. De-duplicate the failure counting logic
**File:** `hxprobe/hxprobe/cli.py`

The identical "dedupe failures into ordered (phase, message, count)" loop appears
twice: `_print_failure_summary` (lines 242–258) and `_build_json_entry`
(lines 314–325). Extract one helper next to the other rendering helpers:

```python
def _summarize_failures(failed: list[Result]) -> list[tuple[str, str, int]]:
    counts: dict[tuple[str, str], int] = {}
    order: list[tuple[str, str]] = []
    for r in failed:
        key = (r.fail_phase, str(r.err))
        if key not in counts:
            order.append(key)
            counts[key] = 0
        counts[key] += 1
    return [(ph, msg, counts[(ph, msg)]) for ph, msg in order]
```

Rewrite both call sites to consume it (text side keeps the `1 ×` vs `N ×`
formatting; JSON side maps to `{"phase", "count", "message"}`).

---

## Explicitly NOT changing (considered, kept)

- **The custom httpcore backend** (`_TimingBackend`/`_TimingStream`/
  `_TimingTransport`) — this *is* the tool's reason to exist (splitting DNS/TCP,
  timing TLS). Not scaffolding.
- **`ThreadPoolExecutor` concurrency** — backs the shipped `-c/--concurrency`
  and multi-URL/`-f` features. Real, not speculative.
- **Explicit per-phase dataclass fields** in `probe.py`/`aggregate.py` — a loop
  would be shorter but less readable; explicit is clearer and matches the text/
  JSON renderers. Leave as-is.
- **`CertInfo.sans`** — not shown in the text block but *is* emitted in JSON
  verbose output; it's a real feature, not dead.

---

## Verification

1. **Unit/CLI tests (primary contract):**
   ```
   cd hxprobe && .venv/bin/python -m pytest tests/test_cli.py tests/test_probe.py -q
   ```
   All must pass with no edits to the test files. These already cover: `-h`
   help→stdout + exit 0, the four usage errors→stderr + exit 1, DNS/connect/
   timeout failures, `--fail`, `--json`, redirects, `--no-http2`,
   `--no-follow-redirects`, `-f` file input, and verbose/cert/TLS detail.

2. **Lint:** `cd hxprobe && .venv/bin/ruff check hxprobe/`

3. **Manual smoke (help + error routing, since #1 rewrites that path):**
   ```
   .venv/bin/python -m hxprobe -h            # help on stdout, exit 0
   .venv/bin/python -m hxprobe               # "no URLs given" on stderr, exit 1
   .venv/bin/python -m hxprobe https://example.com -v   # phases + verbose block
   ```

4. **Per-CLAUDE.md project conventions:** after implementing, add a
   `docs/summaries/<yyyy-mm-dd-hh-mm>-hxprobe-simplification.md` summary and a
   `CHANGELOG.md` entry. (No `docs/usage/` change — behavior is unchanged.)