cli: the tty-vs-piped styling gate covers four commands; run, trace and metrics are styled but untested #17

Description

@Shashankss1205

Summary

The styling contract — ANSI-escape-stripped tty output must equal piped output, byte for byte — is gated by tests/test_cli_style.py, but only for four commands. PR #11 shipped the restyle with this noted as a known gap ("proven ad-hoc but not committed"); the committed gate closed it for a subset:

# Commands with styled human-mode output. Exit codes are deliberately not pinned# here: `models --check` exits 1 when the host can reach no real provider, which# is correct and is what a machine with no credentials does.STYLED= [
pytest.param(["plan", "investigate the checkout outage"], id="plan"),
pytest.param(["models"], id="models"),
pytest.param(["models", "--check"], id="models-check"),
pytest.param(["demo", "stage0"], id="demo-stage0"),
]

Every other styled command is outside the gate. grapharc run prints tinted ADMITTED/REFUSED verdict blocks (grapharc/cli/graphrun.py:204-286), grapharc trace paints every event row (grapharc/cli/main.py:496-504), grapharc metrics styles its kv_block (grapharc/cli/main.py:521-526) — none is covered by test_a_terminal_gets_escapes_and_a_pipe_gets_none or test_stripping_the_escapes_reproduces_the_piped_output_exactly. A call site in any of them that painted unconditionally, or a tty-only layout change, would leak escapes into pipes (or make the docs describe output nobody sees) and no test would notice.

Why this matters

The piped bytes of these commands are load-bearing, not cosmetic: tests/test_readme.py and the cookbook tests byte-compare a subset of command output against fenced doc blocks, and CI consumes run --check-only as a linter. The style module's whole safety argument (grapharc/cli/style.py docstring: "a pipe gets exactly the bytes it got before this module existed") is currently enforced by tests for plan, models and demo stage0 only. The commands most likely to be piped or captured — run in CI, trace | grep, metrics in a script — are exactly the uncovered ones, and run's REFUSED path styles both the key and the value of its verdict line, a shape no covered command exercises.

Where in the code

  • tests/test_cli_style.py:36-49STYLED and COMPARABLE stop at four commands; the pty/pipe machinery and _TMPDIR normalisation already in this file are everything the new cases need
  • grapharc/cli/graphrun.py:204-286 — styled run output: header, ADMITTED/REFUSED verdicts, tinted rejection rows — uncovered
  • grapharc/cli/main.py:496-504 — styled trace rows (style.dim, style.cell, style.accent, style.err) — uncovered
  • grapharc/cli/main.py:521-526 — styled metrics output — uncovered

Confirm the gap for yourself — run --check-only emits escapes on a tty that nothing asserts on:

uv run pytest tests/test_cli_style.py -q --collect-only # 4 styled ids: plan, models, models-check, demo-stage0printf'{"nodes": [{"name": "triage"}], "edges": [{"source": "__start__", "target": "triage"}, {"source": "triage", "target": "__end__"}]}\n'> /tmp/topo.json
script -qec "uv run python -m grapharc.cli.main run /tmp/topo.json --check-only" /dev/null | cat -v | grep '\^\['# ^[[38;5;… escapes are emitted on a tty; no test compares them against the piped form

What to change

  1. Add hermetic cases to STYLED/COMPARABLE in tests/test_cli_style.py for the uncovered styled commands: run <topo> --check-only (admitted path), run <bad-topo> (REFUSED path, which styles key and value tints no current case reaches), trace <trace.jsonl>, and metrics <trace.jsonl> <run-id>. A fixture can write the topology file and produce the trace with one demo stage0 --trace run, the way test_viz_on_a_terminal_stays_pasteable_mermaid already does.
  2. Keep the existing judgement calls: exit codes stay unpinned, and any command whose output varies run-to-run stays out of COMPARABLE (the run refused/admitted text is deterministic apart from the mkdtemp trace path, which the existing _TMPDIR normaliser already handles).
  3. If parametrising requires per-case setup (a file argument), a small indirection — building the argv in the test from a tmp_path-scoped fixture — is fine; do not weaken the assertion (stripped tty output must equal piped stdout+stderr exactly).

Out of scope: serve (blocks on a socket), demo --model live runs and agent (need a real provider), replay/diff (their text is the engine formatter's, deliberately unstyled — see grapharc/cli/main.py and grapharc/cli/replay.py), and any change to grapharc/cli/style.py itself.

How to verify

uv run pytest -q
uv run ruff check .

The new parametrised cases in tests/test_cli_style.py are the proof. Check they can go red: add a stray style.ok(...) guarded by nothing (or an unconditional "\x1b[32m") to one line of the run verdict block in grapharc/cli/graphrun.py and the new run cases must fail while the existing four stay green — that is precisely the blind spot being closed.

Acceptance criteria

  • run (admitted and refused), trace and metrics are covered by both the escapes-on-a-tty/none-in-a-pipe test and the strip-equality test
  • The new cases are hermetic: no network, no credentials, no reliance on the host's terminal, and they pass on a machine with no provider configured
  • Existing four cases and their ids are unchanged
  • uv run pytest stays green and uv run ruff check . is clean
  • Any README or cookbook sentence this changes is updated in the same pull request (several are byte-compared against real output by the test suite)

Skill level

good first issue — the pty harness, environment scrubbing and path normalisation are already written in the same file; the work is producing fixtures for four more argv lists and wiring them into the existing parametrisation. The one subtlety is keeping the cases deterministic (fixture files, not fresh mkdtemp paths, wherever the output embeds one) — copy how test_viz_on_a_terminal_stays_pasteable_mermaid obtains its trace. Questions welcome on the issue.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
       blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
      }
      } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
      })();
      (function(){
      try {
      var __m = "github.com";
      var __re = new RegExp('^' + "github\\.com" + '
      
      Skip to content

      cli: the tty-vs-piped styling gate covers four commands; run, trace and metrics are styled but untested #17

      Description

      @Shashankss1205

      Summary

      The styling contract — ANSI-escape-stripped tty output must equal piped output, byte for byte — is gated by tests/test_cli_style.py, but only for four commands. PR #11 shipped the restyle with this noted as a known gap ("proven ad-hoc but not committed"); the committed gate closed it for a subset:

      # Commands with styled human-mode output. Exit codes are deliberately not pinned# here: `models --check` exits 1 when the host can reach no real provider, which# is correct and is what a machine with no credentials does.STYLED= [
      pytest.param(["plan", "investigate the checkout outage"], id="plan"),
      pytest.param(["models"], id="models"),
      pytest.param(["models", "--check"], id="models-check"),
      pytest.param(["demo", "stage0"], id="demo-stage0"),
      ]

      Every other styled command is outside the gate. grapharc run prints tinted ADMITTED/REFUSED verdict blocks (grapharc/cli/graphrun.py:204-286), grapharc trace paints every event row (grapharc/cli/main.py:496-504), grapharc metrics styles its kv_block (grapharc/cli/main.py:521-526) — none is covered by test_a_terminal_gets_escapes_and_a_pipe_gets_none or test_stripping_the_escapes_reproduces_the_piped_output_exactly. A call site in any of them that painted unconditionally, or a tty-only layout change, would leak escapes into pipes (or make the docs describe output nobody sees) and no test would notice.

      Why this matters

      The piped bytes of these commands are load-bearing, not cosmetic: tests/test_readme.py and the cookbook tests byte-compare a subset of command output against fenced doc blocks, and CI consumes run --check-only as a linter. The style module's whole safety argument (grapharc/cli/style.py docstring: "a pipe gets exactly the bytes it got before this module existed") is currently enforced by tests for plan, models and demo stage0 only. The commands most likely to be piped or captured — run in CI, trace | grep, metrics in a script — are exactly the uncovered ones, and run's REFUSED path styles both the key and the value of its verdict line, a shape no covered command exercises.

      Where in the code

      • tests/test_cli_style.py:36-49STYLED and COMPARABLE stop at four commands; the pty/pipe machinery and _TMPDIR normalisation already in this file are everything the new cases need
      • grapharc/cli/graphrun.py:204-286 — styled run output: header, ADMITTED/REFUSED verdicts, tinted rejection rows — uncovered
      • grapharc/cli/main.py:496-504 — styled trace rows (style.dim, style.cell, style.accent, style.err) — uncovered
      • grapharc/cli/main.py:521-526 — styled metrics output — uncovered

      Confirm the gap for yourself — run --check-only emits escapes on a tty that nothing asserts on:

      uv run pytest tests/test_cli_style.py -q --collect-only # 4 styled ids: plan, models, models-check, demo-stage0printf'{"nodes": [{"name": "triage"}], "edges": [{"source": "__start__", "target": "triage"}, {"source": "triage", "target": "__end__"}]}\n'> /tmp/topo.json
      script -qec "uv run python -m grapharc.cli.main run /tmp/topo.json --check-only" /dev/null | cat -v | grep '\^\['# ^[[38;5;… escapes are emitted on a tty; no test compares them against the piped form

      What to change

      1. Add hermetic cases to STYLED/COMPARABLE in tests/test_cli_style.py for the uncovered styled commands: run <topo> --check-only (admitted path), run <bad-topo> (REFUSED path, which styles key and value tints no current case reaches), trace <trace.jsonl>, and metrics <trace.jsonl> <run-id>. A fixture can write the topology file and produce the trace with one demo stage0 --trace run, the way test_viz_on_a_terminal_stays_pasteable_mermaid already does.
      2. Keep the existing judgement calls: exit codes stay unpinned, and any command whose output varies run-to-run stays out of COMPARABLE (the run refused/admitted text is deterministic apart from the mkdtemp trace path, which the existing _TMPDIR normaliser already handles).
      3. If parametrising requires per-case setup (a file argument), a small indirection — building the argv in the test from a tmp_path-scoped fixture — is fine; do not weaken the assertion (stripped tty output must equal piped stdout+stderr exactly).

      Out of scope: serve (blocks on a socket), demo --model live runs and agent (need a real provider), replay/diff (their text is the engine formatter's, deliberately unstyled — see grapharc/cli/main.py and grapharc/cli/replay.py), and any change to grapharc/cli/style.py itself.

      How to verify

      uv run pytest -q
      uv run ruff check .

      The new parametrised cases in tests/test_cli_style.py are the proof. Check they can go red: add a stray style.ok(...) guarded by nothing (or an unconditional "\x1b[32m") to one line of the run verdict block in grapharc/cli/graphrun.py and the new run cases must fail while the existing four stay green — that is precisely the blind spot being closed.

      Acceptance criteria

      • run (admitted and refused), trace and metrics are covered by both the escapes-on-a-tty/none-in-a-pipe test and the strip-equality test
      • The new cases are hermetic: no network, no credentials, no reliance on the host's terminal, and they pass on a machine with no provider configured
      • Existing four cases and their ids are unchanged
      • uv run pytest stays green and uv run ruff check . is clean
      • Any README or cookbook sentence this changes is updated in the same pull request (several are byte-compared against real output by the test suite)

      Skill level

      good first issue — the pty harness, environment scrubbing and path normalisation are already written in the same file; the work is producing fixtures for four more argv lists and wiring them into the existing parametrisation. The one subtlety is keeping the cases deterministic (fixture files, not fresh mkdtemp paths, wherever the output embeds one) — copy how test_viz_on_a_terminal_stays_pasteable_mermaid obtains its trace. Questions welcome on the issue.

      Activity

      Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

      Metadata

      Metadata

      Assignees

      No one assigned

        Labels

        No labels
        No labels

        Type

        No type

        Projects

        No projects

          Milestone

          No milestone

          Relationships

          None yet

          Development

          No branches or pull requests

          Issue actions

          , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
          Skip to content

          cli: the tty-vs-piped styling gate covers four commands; run, trace and metrics are styled but untested #17

          Description

          @Shashankss1205

          Summary

          The styling contract — ANSI-escape-stripped tty output must equal piped output, byte for byte — is gated by tests/test_cli_style.py, but only for four commands. PR #11 shipped the restyle with this noted as a known gap ("proven ad-hoc but not committed"); the committed gate closed it for a subset:

          # Commands with styled human-mode output. Exit codes are deliberately not pinned# here: `models --check` exits 1 when the host can reach no real provider, which# is correct and is what a machine with no credentials does.STYLED= [
          pytest.param(["plan", "investigate the checkout outage"], id="plan"),
          pytest.param(["models"], id="models"),
          pytest.param(["models", "--check"], id="models-check"),
          pytest.param(["demo", "stage0"], id="demo-stage0"),
          ]

          Every other styled command is outside the gate. grapharc run prints tinted ADMITTED/REFUSED verdict blocks (grapharc/cli/graphrun.py:204-286), grapharc trace paints every event row (grapharc/cli/main.py:496-504), grapharc metrics styles its kv_block (grapharc/cli/main.py:521-526) — none is covered by test_a_terminal_gets_escapes_and_a_pipe_gets_none or test_stripping_the_escapes_reproduces_the_piped_output_exactly. A call site in any of them that painted unconditionally, or a tty-only layout change, would leak escapes into pipes (or make the docs describe output nobody sees) and no test would notice.

          Why this matters

          The piped bytes of these commands are load-bearing, not cosmetic: tests/test_readme.py and the cookbook tests byte-compare a subset of command output against fenced doc blocks, and CI consumes run --check-only as a linter. The style module's whole safety argument (grapharc/cli/style.py docstring: "a pipe gets exactly the bytes it got before this module existed") is currently enforced by tests for plan, models and demo stage0 only. The commands most likely to be piped or captured — run in CI, trace | grep, metrics in a script — are exactly the uncovered ones, and run's REFUSED path styles both the key and the value of its verdict line, a shape no covered command exercises.

          Where in the code

          • tests/test_cli_style.py:36-49STYLED and COMPARABLE stop at four commands; the pty/pipe machinery and _TMPDIR normalisation already in this file are everything the new cases need
          • grapharc/cli/graphrun.py:204-286 — styled run output: header, ADMITTED/REFUSED verdicts, tinted rejection rows — uncovered
          • grapharc/cli/main.py:496-504 — styled trace rows (style.dim, style.cell, style.accent, style.err) — uncovered
          • grapharc/cli/main.py:521-526 — styled metrics output — uncovered

          Confirm the gap for yourself — run --check-only emits escapes on a tty that nothing asserts on:

          uv run pytest tests/test_cli_style.py -q --collect-only # 4 styled ids: plan, models, models-check, demo-stage0printf'{"nodes": [{"name": "triage"}], "edges": [{"source": "__start__", "target": "triage"}, {"source": "triage", "target": "__end__"}]}\n'> /tmp/topo.json
          script -qec "uv run python -m grapharc.cli.main run /tmp/topo.json --check-only" /dev/null | cat -v | grep '\^\['# ^[[38;5;… escapes are emitted on a tty; no test compares them against the piped form

          What to change

          1. Add hermetic cases to STYLED/COMPARABLE in tests/test_cli_style.py for the uncovered styled commands: run <topo> --check-only (admitted path), run <bad-topo> (REFUSED path, which styles key and value tints no current case reaches), trace <trace.jsonl>, and metrics <trace.jsonl> <run-id>. A fixture can write the topology file and produce the trace with one demo stage0 --trace run, the way test_viz_on_a_terminal_stays_pasteable_mermaid already does.
          2. Keep the existing judgement calls: exit codes stay unpinned, and any command whose output varies run-to-run stays out of COMPARABLE (the run refused/admitted text is deterministic apart from the mkdtemp trace path, which the existing _TMPDIR normaliser already handles).
          3. If parametrising requires per-case setup (a file argument), a small indirection — building the argv in the test from a tmp_path-scoped fixture — is fine; do not weaken the assertion (stripped tty output must equal piped stdout+stderr exactly).

          Out of scope: serve (blocks on a socket), demo --model live runs and agent (need a real provider), replay/diff (their text is the engine formatter's, deliberately unstyled — see grapharc/cli/main.py and grapharc/cli/replay.py), and any change to grapharc/cli/style.py itself.

          How to verify

          uv run pytest -q
          uv run ruff check .

          The new parametrised cases in tests/test_cli_style.py are the proof. Check they can go red: add a stray style.ok(...) guarded by nothing (or an unconditional "\x1b[32m") to one line of the run verdict block in grapharc/cli/graphrun.py and the new run cases must fail while the existing four stay green — that is precisely the blind spot being closed.

          Acceptance criteria

          • run (admitted and refused), trace and metrics are covered by both the escapes-on-a-tty/none-in-a-pipe test and the strip-equality test
          • The new cases are hermetic: no network, no credentials, no reliance on the host's terminal, and they pass on a machine with no provider configured
          • Existing four cases and their ids are unchanged
          • uv run pytest stays green and uv run ruff check . is clean
          • Any README or cookbook sentence this changes is updated in the same pull request (several are byte-compared against real output by the test suite)

          Skill level

          good first issue — the pty harness, environment scrubbing and path normalisation are already written in the same file; the work is producing fixtures for four more argv lists and wiring them into the existing parametrisation. The one subtlety is keeping the cases deterministic (fixture files, not fresh mkdtemp paths, wherever the output embeds one) — copy how test_viz_on_a_terminal_stays_pasteable_mermaid obtains its trace. Questions welcome on the issue.

          Activity

          Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

          Metadata

          Metadata

          Assignees

          No one assigned

            Labels

            No labels
            No labels

            Type

            No type

            Projects

            No projects

              Milestone

              No milestone

              Relationships

              None yet

              Development

              No branches or pull requests

              Issue actions

              , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
              Skip to content

              cli: the tty-vs-piped styling gate covers four commands; run, trace and metrics are styled but untested #17

              Description

              @Shashankss1205

              Summary

              The styling contract — ANSI-escape-stripped tty output must equal piped output, byte for byte — is gated by tests/test_cli_style.py, but only for four commands. PR #11 shipped the restyle with this noted as a known gap ("proven ad-hoc but not committed"); the committed gate closed it for a subset:

              # Commands with styled human-mode output. Exit codes are deliberately not pinned# here: `models --check` exits 1 when the host can reach no real provider, which# is correct and is what a machine with no credentials does.STYLED= [
              pytest.param(["plan", "investigate the checkout outage"], id="plan"),
              pytest.param(["models"], id="models"),
              pytest.param(["models", "--check"], id="models-check"),
              pytest.param(["demo", "stage0"], id="demo-stage0"),
              ]

              Every other styled command is outside the gate. grapharc run prints tinted ADMITTED/REFUSED verdict blocks (grapharc/cli/graphrun.py:204-286), grapharc trace paints every event row (grapharc/cli/main.py:496-504), grapharc metrics styles its kv_block (grapharc/cli/main.py:521-526) — none is covered by test_a_terminal_gets_escapes_and_a_pipe_gets_none or test_stripping_the_escapes_reproduces_the_piped_output_exactly. A call site in any of them that painted unconditionally, or a tty-only layout change, would leak escapes into pipes (or make the docs describe output nobody sees) and no test would notice.

              Why this matters

              The piped bytes of these commands are load-bearing, not cosmetic: tests/test_readme.py and the cookbook tests byte-compare a subset of command output against fenced doc blocks, and CI consumes run --check-only as a linter. The style module's whole safety argument (grapharc/cli/style.py docstring: "a pipe gets exactly the bytes it got before this module existed") is currently enforced by tests for plan, models and demo stage0 only. The commands most likely to be piped or captured — run in CI, trace | grep, metrics in a script — are exactly the uncovered ones, and run's REFUSED path styles both the key and the value of its verdict line, a shape no covered command exercises.

              Where in the code

              • tests/test_cli_style.py:36-49STYLED and COMPARABLE stop at four commands; the pty/pipe machinery and _TMPDIR normalisation already in this file are everything the new cases need
              • grapharc/cli/graphrun.py:204-286 — styled run output: header, ADMITTED/REFUSED verdicts, tinted rejection rows — uncovered
              • grapharc/cli/main.py:496-504 — styled trace rows (style.dim, style.cell, style.accent, style.err) — uncovered
              • grapharc/cli/main.py:521-526 — styled metrics output — uncovered

              Confirm the gap for yourself — run --check-only emits escapes on a tty that nothing asserts on:

              uv run pytest tests/test_cli_style.py -q --collect-only # 4 styled ids: plan, models, models-check, demo-stage0printf'{"nodes": [{"name": "triage"}], "edges": [{"source": "__start__", "target": "triage"}, {"source": "triage", "target": "__end__"}]}\n'> /tmp/topo.json
              script -qec "uv run python -m grapharc.cli.main run /tmp/topo.json --check-only" /dev/null | cat -v | grep '\^\['# ^[[38;5;… escapes are emitted on a tty; no test compares them against the piped form

              What to change

              1. Add hermetic cases to STYLED/COMPARABLE in tests/test_cli_style.py for the uncovered styled commands: run <topo> --check-only (admitted path), run <bad-topo> (REFUSED path, which styles key and value tints no current case reaches), trace <trace.jsonl>, and metrics <trace.jsonl> <run-id>. A fixture can write the topology file and produce the trace with one demo stage0 --trace run, the way test_viz_on_a_terminal_stays_pasteable_mermaid already does.
              2. Keep the existing judgement calls: exit codes stay unpinned, and any command whose output varies run-to-run stays out of COMPARABLE (the run refused/admitted text is deterministic apart from the mkdtemp trace path, which the existing _TMPDIR normaliser already handles).
              3. If parametrising requires per-case setup (a file argument), a small indirection — building the argv in the test from a tmp_path-scoped fixture — is fine; do not weaken the assertion (stripped tty output must equal piped stdout+stderr exactly).

              Out of scope: serve (blocks on a socket), demo --model live runs and agent (need a real provider), replay/diff (their text is the engine formatter's, deliberately unstyled — see grapharc/cli/main.py and grapharc/cli/replay.py), and any change to grapharc/cli/style.py itself.

              How to verify

              uv run pytest -q
              uv run ruff check .

              The new parametrised cases in tests/test_cli_style.py are the proof. Check they can go red: add a stray style.ok(...) guarded by nothing (or an unconditional "\x1b[32m") to one line of the run verdict block in grapharc/cli/graphrun.py and the new run cases must fail while the existing four stay green — that is precisely the blind spot being closed.

              Acceptance criteria

              • run (admitted and refused), trace and metrics are covered by both the escapes-on-a-tty/none-in-a-pipe test and the strip-equality test
              • The new cases are hermetic: no network, no credentials, no reliance on the host's terminal, and they pass on a machine with no provider configured
              • Existing four cases and their ids are unchanged
              • uv run pytest stays green and uv run ruff check . is clean
              • Any README or cookbook sentence this changes is updated in the same pull request (several are byte-compared against real output by the test suite)

              Skill level

              good first issue — the pty harness, environment scrubbing and path normalisation are already written in the same file; the work is producing fixtures for four more argv lists and wiring them into the existing parametrisation. The one subtlety is keeping the cases deterministic (fixture files, not fresh mkdtemp paths, wherever the output embeds one) — copy how test_viz_on_a_terminal_stays_pasteable_mermaid obtains its trace. Questions welcome on the issue.

              Activity

              Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

              Metadata

              Metadata

              Assignees

              No one assigned

                Labels

                No labels
                No labels

                Type

                No type

                Projects

                No projects

                  Milestone

                  No milestone

                  Relationships

                  None yet

                  Development

                  No branches or pull requests

                  Issue actions

                  , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
                  Skip to content

                  cli: the tty-vs-piped styling gate covers four commands; run, trace and metrics are styled but untested #17

                  Description

                  @Shashankss1205

                  Summary

                  The styling contract — ANSI-escape-stripped tty output must equal piped output, byte for byte — is gated by tests/test_cli_style.py, but only for four commands. PR #11 shipped the restyle with this noted as a known gap ("proven ad-hoc but not committed"); the committed gate closed it for a subset:

                  # Commands with styled human-mode output. Exit codes are deliberately not pinned# here: `models --check` exits 1 when the host can reach no real provider, which# is correct and is what a machine with no credentials does.STYLED= [
                  pytest.param(["plan", "investigate the checkout outage"], id="plan"),
                  pytest.param(["models"], id="models"),
                  pytest.param(["models", "--check"], id="models-check"),
                  pytest.param(["demo", "stage0"], id="demo-stage0"),
                  ]

                  Every other styled command is outside the gate. grapharc run prints tinted ADMITTED/REFUSED verdict blocks (grapharc/cli/graphrun.py:204-286), grapharc trace paints every event row (grapharc/cli/main.py:496-504), grapharc metrics styles its kv_block (grapharc/cli/main.py:521-526) — none is covered by test_a_terminal_gets_escapes_and_a_pipe_gets_none or test_stripping_the_escapes_reproduces_the_piped_output_exactly. A call site in any of them that painted unconditionally, or a tty-only layout change, would leak escapes into pipes (or make the docs describe output nobody sees) and no test would notice.

                  Why this matters

                  The piped bytes of these commands are load-bearing, not cosmetic: tests/test_readme.py and the cookbook tests byte-compare a subset of command output against fenced doc blocks, and CI consumes run --check-only as a linter. The style module's whole safety argument (grapharc/cli/style.py docstring: "a pipe gets exactly the bytes it got before this module existed") is currently enforced by tests for plan, models and demo stage0 only. The commands most likely to be piped or captured — run in CI, trace | grep, metrics in a script — are exactly the uncovered ones, and run's REFUSED path styles both the key and the value of its verdict line, a shape no covered command exercises.

                  Where in the code

                  • tests/test_cli_style.py:36-49STYLED and COMPARABLE stop at four commands; the pty/pipe machinery and _TMPDIR normalisation already in this file are everything the new cases need
                  • grapharc/cli/graphrun.py:204-286 — styled run output: header, ADMITTED/REFUSED verdicts, tinted rejection rows — uncovered
                  • grapharc/cli/main.py:496-504 — styled trace rows (style.dim, style.cell, style.accent, style.err) — uncovered
                  • grapharc/cli/main.py:521-526 — styled metrics output — uncovered

                  Confirm the gap for yourself — run --check-only emits escapes on a tty that nothing asserts on:

                  uv run pytest tests/test_cli_style.py -q --collect-only # 4 styled ids: plan, models, models-check, demo-stage0printf'{"nodes": [{"name": "triage"}], "edges": [{"source": "__start__", "target": "triage"}, {"source": "triage", "target": "__end__"}]}\n'> /tmp/topo.json
                  script -qec "uv run python -m grapharc.cli.main run /tmp/topo.json --check-only" /dev/null | cat -v | grep '\^\['# ^[[38;5;… escapes are emitted on a tty; no test compares them against the piped form

                  What to change

                  1. Add hermetic cases to STYLED/COMPARABLE in tests/test_cli_style.py for the uncovered styled commands: run <topo> --check-only (admitted path), run <bad-topo> (REFUSED path, which styles key and value tints no current case reaches), trace <trace.jsonl>, and metrics <trace.jsonl> <run-id>. A fixture can write the topology file and produce the trace with one demo stage0 --trace run, the way test_viz_on_a_terminal_stays_pasteable_mermaid already does.
                  2. Keep the existing judgement calls: exit codes stay unpinned, and any command whose output varies run-to-run stays out of COMPARABLE (the run refused/admitted text is deterministic apart from the mkdtemp trace path, which the existing _TMPDIR normaliser already handles).
                  3. If parametrising requires per-case setup (a file argument), a small indirection — building the argv in the test from a tmp_path-scoped fixture — is fine; do not weaken the assertion (stripped tty output must equal piped stdout+stderr exactly).

                  Out of scope: serve (blocks on a socket), demo --model live runs and agent (need a real provider), replay/diff (their text is the engine formatter's, deliberately unstyled — see grapharc/cli/main.py and grapharc/cli/replay.py), and any change to grapharc/cli/style.py itself.

                  How to verify

                  uv run pytest -q
                  uv run ruff check .

                  The new parametrised cases in tests/test_cli_style.py are the proof. Check they can go red: add a stray style.ok(...) guarded by nothing (or an unconditional "\x1b[32m") to one line of the run verdict block in grapharc/cli/graphrun.py and the new run cases must fail while the existing four stay green — that is precisely the blind spot being closed.

                  Acceptance criteria

                  • run (admitted and refused), trace and metrics are covered by both the escapes-on-a-tty/none-in-a-pipe test and the strip-equality test
                  • The new cases are hermetic: no network, no credentials, no reliance on the host's terminal, and they pass on a machine with no provider configured
                  • Existing four cases and their ids are unchanged
                  • uv run pytest stays green and uv run ruff check . is clean
                  • Any README or cookbook sentence this changes is updated in the same pull request (several are byte-compared against real output by the test suite)

                  Skill level

                  good first issue — the pty harness, environment scrubbing and path normalisation are already written in the same file; the work is producing fixtures for four more argv lists and wiring them into the existing parametrisation. The one subtlety is keeping the cases deterministic (fixture files, not fresh mkdtemp paths, wherever the output embeds one) — copy how test_viz_on_a_terminal_stays_pasteable_mermaid obtains its trace. Questions welcome on the issue.

                  Activity

                  Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

                  Metadata

                  Metadata

                  Assignees

                  No one assigned

                    Labels

                    No labels
                    No labels

                    Type

                    No type

                    Projects

                    No projects

                      Milestone

                      No milestone

                      Relationships

                      None yet

                      Development

                      No branches or pull requests

                      Issue actions

                      , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
                      Skip to content

                      cli: the tty-vs-piped styling gate covers four commands; run, trace and metrics are styled but untested #17

                      Description

                      @Shashankss1205

                      Summary

                      The styling contract — ANSI-escape-stripped tty output must equal piped output, byte for byte — is gated by tests/test_cli_style.py, but only for four commands. PR #11 shipped the restyle with this noted as a known gap ("proven ad-hoc but not committed"); the committed gate closed it for a subset:

                      # Commands with styled human-mode output. Exit codes are deliberately not pinned# here: `models --check` exits 1 when the host can reach no real provider, which# is correct and is what a machine with no credentials does.STYLED= [
                      pytest.param(["plan", "investigate the checkout outage"], id="plan"),
                      pytest.param(["models"], id="models"),
                      pytest.param(["models", "--check"], id="models-check"),
                      pytest.param(["demo", "stage0"], id="demo-stage0"),
                      ]

                      Every other styled command is outside the gate. grapharc run prints tinted ADMITTED/REFUSED verdict blocks (grapharc/cli/graphrun.py:204-286), grapharc trace paints every event row (grapharc/cli/main.py:496-504), grapharc metrics styles its kv_block (grapharc/cli/main.py:521-526) — none is covered by test_a_terminal_gets_escapes_and_a_pipe_gets_none or test_stripping_the_escapes_reproduces_the_piped_output_exactly. A call site in any of them that painted unconditionally, or a tty-only layout change, would leak escapes into pipes (or make the docs describe output nobody sees) and no test would notice.

                      Why this matters

                      The piped bytes of these commands are load-bearing, not cosmetic: tests/test_readme.py and the cookbook tests byte-compare a subset of command output against fenced doc blocks, and CI consumes run --check-only as a linter. The style module's whole safety argument (grapharc/cli/style.py docstring: "a pipe gets exactly the bytes it got before this module existed") is currently enforced by tests for plan, models and demo stage0 only. The commands most likely to be piped or captured — run in CI, trace | grep, metrics in a script — are exactly the uncovered ones, and run's REFUSED path styles both the key and the value of its verdict line, a shape no covered command exercises.

                      Where in the code

                      • tests/test_cli_style.py:36-49STYLED and COMPARABLE stop at four commands; the pty/pipe machinery and _TMPDIR normalisation already in this file are everything the new cases need
                      • grapharc/cli/graphrun.py:204-286 — styled run output: header, ADMITTED/REFUSED verdicts, tinted rejection rows — uncovered
                      • grapharc/cli/main.py:496-504 — styled trace rows (style.dim, style.cell, style.accent, style.err) — uncovered
                      • grapharc/cli/main.py:521-526 — styled metrics output — uncovered

                      Confirm the gap for yourself — run --check-only emits escapes on a tty that nothing asserts on:

                      uv run pytest tests/test_cli_style.py -q --collect-only # 4 styled ids: plan, models, models-check, demo-stage0printf'{"nodes": [{"name": "triage"}], "edges": [{"source": "__start__", "target": "triage"}, {"source": "triage", "target": "__end__"}]}\n'> /tmp/topo.json
                      script -qec "uv run python -m grapharc.cli.main run /tmp/topo.json --check-only" /dev/null | cat -v | grep '\^\['# ^[[38;5;… escapes are emitted on a tty; no test compares them against the piped form

                      What to change

                      1. Add hermetic cases to STYLED/COMPARABLE in tests/test_cli_style.py for the uncovered styled commands: run <topo> --check-only (admitted path), run <bad-topo> (REFUSED path, which styles key and value tints no current case reaches), trace <trace.jsonl>, and metrics <trace.jsonl> <run-id>. A fixture can write the topology file and produce the trace with one demo stage0 --trace run, the way test_viz_on_a_terminal_stays_pasteable_mermaid already does.
                      2. Keep the existing judgement calls: exit codes stay unpinned, and any command whose output varies run-to-run stays out of COMPARABLE (the run refused/admitted text is deterministic apart from the mkdtemp trace path, which the existing _TMPDIR normaliser already handles).
                      3. If parametrising requires per-case setup (a file argument), a small indirection — building the argv in the test from a tmp_path-scoped fixture — is fine; do not weaken the assertion (stripped tty output must equal piped stdout+stderr exactly).

                      Out of scope: serve (blocks on a socket), demo --model live runs and agent (need a real provider), replay/diff (their text is the engine formatter's, deliberately unstyled — see grapharc/cli/main.py and grapharc/cli/replay.py), and any change to grapharc/cli/style.py itself.

                      How to verify

                      uv run pytest -q
                      uv run ruff check .

                      The new parametrised cases in tests/test_cli_style.py are the proof. Check they can go red: add a stray style.ok(...) guarded by nothing (or an unconditional "\x1b[32m") to one line of the run verdict block in grapharc/cli/graphrun.py and the new run cases must fail while the existing four stay green — that is precisely the blind spot being closed.

                      Acceptance criteria

                      • run (admitted and refused), trace and metrics are covered by both the escapes-on-a-tty/none-in-a-pipe test and the strip-equality test
                      • The new cases are hermetic: no network, no credentials, no reliance on the host's terminal, and they pass on a machine with no provider configured
                      • Existing four cases and their ids are unchanged
                      • uv run pytest stays green and uv run ruff check . is clean
                      • Any README or cookbook sentence this changes is updated in the same pull request (several are byte-compared against real output by the test suite)

                      Skill level

                      good first issue — the pty harness, environment scrubbing and path normalisation are already written in the same file; the work is producing fixtures for four more argv lists and wiring them into the existing parametrisation. The one subtlety is keeping the cases deterministic (fixture files, not fresh mkdtemp paths, wherever the output embeds one) — copy how test_viz_on_a_terminal_stays_pasteable_mermaid obtains its trace. Questions welcome on the issue.

                      Activity

                      Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

                      Metadata

                      Metadata

                      Assignees

                      No one assigned

                        Labels

                        No labels
                        No labels

                        Type

                        No type

                        Projects

                        No projects

                          Milestone

                          No milestone

                          Relationships

                          None yet

                          Development

                          No branches or pull requests

                          Issue actions

                          , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
                          Skip to content

                          cli: the tty-vs-piped styling gate covers four commands; run, trace and metrics are styled but untested #17

                          Description

                          @Shashankss1205

                          Summary

                          The styling contract — ANSI-escape-stripped tty output must equal piped output, byte for byte — is gated by tests/test_cli_style.py, but only for four commands. PR #11 shipped the restyle with this noted as a known gap ("proven ad-hoc but not committed"); the committed gate closed it for a subset:

                          # Commands with styled human-mode output. Exit codes are deliberately not pinned# here: `models --check` exits 1 when the host can reach no real provider, which# is correct and is what a machine with no credentials does.STYLED= [
                          pytest.param(["plan", "investigate the checkout outage"], id="plan"),
                          pytest.param(["models"], id="models"),
                          pytest.param(["models", "--check"], id="models-check"),
                          pytest.param(["demo", "stage0"], id="demo-stage0"),
                          ]

                          Every other styled command is outside the gate. grapharc run prints tinted ADMITTED/REFUSED verdict blocks (grapharc/cli/graphrun.py:204-286), grapharc trace paints every event row (grapharc/cli/main.py:496-504), grapharc metrics styles its kv_block (grapharc/cli/main.py:521-526) — none is covered by test_a_terminal_gets_escapes_and_a_pipe_gets_none or test_stripping_the_escapes_reproduces_the_piped_output_exactly. A call site in any of them that painted unconditionally, or a tty-only layout change, would leak escapes into pipes (or make the docs describe output nobody sees) and no test would notice.

                          Why this matters

                          The piped bytes of these commands are load-bearing, not cosmetic: tests/test_readme.py and the cookbook tests byte-compare a subset of command output against fenced doc blocks, and CI consumes run --check-only as a linter. The style module's whole safety argument (grapharc/cli/style.py docstring: "a pipe gets exactly the bytes it got before this module existed") is currently enforced by tests for plan, models and demo stage0 only. The commands most likely to be piped or captured — run in CI, trace | grep, metrics in a script — are exactly the uncovered ones, and run's REFUSED path styles both the key and the value of its verdict line, a shape no covered command exercises.

                          Where in the code

                          • tests/test_cli_style.py:36-49STYLED and COMPARABLE stop at four commands; the pty/pipe machinery and _TMPDIR normalisation already in this file are everything the new cases need
                          • grapharc/cli/graphrun.py:204-286 — styled run output: header, ADMITTED/REFUSED verdicts, tinted rejection rows — uncovered
                          • grapharc/cli/main.py:496-504 — styled trace rows (style.dim, style.cell, style.accent, style.err) — uncovered
                          • grapharc/cli/main.py:521-526 — styled metrics output — uncovered

                          Confirm the gap for yourself — run --check-only emits escapes on a tty that nothing asserts on:

                          uv run pytest tests/test_cli_style.py -q --collect-only # 4 styled ids: plan, models, models-check, demo-stage0printf'{"nodes": [{"name": "triage"}], "edges": [{"source": "__start__", "target": "triage"}, {"source": "triage", "target": "__end__"}]}\n'> /tmp/topo.json
                          script -qec "uv run python -m grapharc.cli.main run /tmp/topo.json --check-only" /dev/null | cat -v | grep '\^\['# ^[[38;5;… escapes are emitted on a tty; no test compares them against the piped form

                          What to change

                          1. Add hermetic cases to STYLED/COMPARABLE in tests/test_cli_style.py for the uncovered styled commands: run <topo> --check-only (admitted path), run <bad-topo> (REFUSED path, which styles key and value tints no current case reaches), trace <trace.jsonl>, and metrics <trace.jsonl> <run-id>. A fixture can write the topology file and produce the trace with one demo stage0 --trace run, the way test_viz_on_a_terminal_stays_pasteable_mermaid already does.
                          2. Keep the existing judgement calls: exit codes stay unpinned, and any command whose output varies run-to-run stays out of COMPARABLE (the run refused/admitted text is deterministic apart from the mkdtemp trace path, which the existing _TMPDIR normaliser already handles).
                          3. If parametrising requires per-case setup (a file argument), a small indirection — building the argv in the test from a tmp_path-scoped fixture — is fine; do not weaken the assertion (stripped tty output must equal piped stdout+stderr exactly).

                          Out of scope: serve (blocks on a socket), demo --model live runs and agent (need a real provider), replay/diff (their text is the engine formatter's, deliberately unstyled — see grapharc/cli/main.py and grapharc/cli/replay.py), and any change to grapharc/cli/style.py itself.

                          How to verify

                          uv run pytest -q
                          uv run ruff check .

                          The new parametrised cases in tests/test_cli_style.py are the proof. Check they can go red: add a stray style.ok(...) guarded by nothing (or an unconditional "\x1b[32m") to one line of the run verdict block in grapharc/cli/graphrun.py and the new run cases must fail while the existing four stay green — that is precisely the blind spot being closed.

                          Acceptance criteria

                          • run (admitted and refused), trace and metrics are covered by both the escapes-on-a-tty/none-in-a-pipe test and the strip-equality test
                          • The new cases are hermetic: no network, no credentials, no reliance on the host's terminal, and they pass on a machine with no provider configured
                          • Existing four cases and their ids are unchanged
                          • uv run pytest stays green and uv run ruff check . is clean
                          • Any README or cookbook sentence this changes is updated in the same pull request (several are byte-compared against real output by the test suite)

                          Skill level

                          good first issue — the pty harness, environment scrubbing and path normalisation are already written in the same file; the work is producing fixtures for four more argv lists and wiring them into the existing parametrisation. The one subtlety is keeping the cases deterministic (fixture files, not fresh mkdtemp paths, wherever the output embeds one) — copy how test_viz_on_a_terminal_stays_pasteable_mermaid obtains its trace. Questions welcome on the issue.

                          Activity

                          Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

                          Metadata

                          Metadata

                          Assignees

                          No one assigned

                            Labels

                            No labels
                            No labels

                            Type

                            No type

                            Projects

                            No projects

                              Milestone

                              No milestone

                              Relationships

                              None yet

                              Development

                              No branches or pull requests

                              Issue actions

                              , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
                              Skip to content

                              cli: the tty-vs-piped styling gate covers four commands; run, trace and metrics are styled but untested #17

                              Description

                              @Shashankss1205

                              Summary

                              The styling contract — ANSI-escape-stripped tty output must equal piped output, byte for byte — is gated by tests/test_cli_style.py, but only for four commands. PR #11 shipped the restyle with this noted as a known gap ("proven ad-hoc but not committed"); the committed gate closed it for a subset:

                              # Commands with styled human-mode output. Exit codes are deliberately not pinned# here: `models --check` exits 1 when the host can reach no real provider, which# is correct and is what a machine with no credentials does.STYLED= [
                              pytest.param(["plan", "investigate the checkout outage"], id="plan"),
                              pytest.param(["models"], id="models"),
                              pytest.param(["models", "--check"], id="models-check"),
                              pytest.param(["demo", "stage0"], id="demo-stage0"),
                              ]

                              Every other styled command is outside the gate. grapharc run prints tinted ADMITTED/REFUSED verdict blocks (grapharc/cli/graphrun.py:204-286), grapharc trace paints every event row (grapharc/cli/main.py:496-504), grapharc metrics styles its kv_block (grapharc/cli/main.py:521-526) — none is covered by test_a_terminal_gets_escapes_and_a_pipe_gets_none or test_stripping_the_escapes_reproduces_the_piped_output_exactly. A call site in any of them that painted unconditionally, or a tty-only layout change, would leak escapes into pipes (or make the docs describe output nobody sees) and no test would notice.

                              Why this matters

                              The piped bytes of these commands are load-bearing, not cosmetic: tests/test_readme.py and the cookbook tests byte-compare a subset of command output against fenced doc blocks, and CI consumes run --check-only as a linter. The style module's whole safety argument (grapharc/cli/style.py docstring: "a pipe gets exactly the bytes it got before this module existed") is currently enforced by tests for plan, models and demo stage0 only. The commands most likely to be piped or captured — run in CI, trace | grep, metrics in a script — are exactly the uncovered ones, and run's REFUSED path styles both the key and the value of its verdict line, a shape no covered command exercises.

                              Where in the code

                              • tests/test_cli_style.py:36-49STYLED and COMPARABLE stop at four commands; the pty/pipe machinery and _TMPDIR normalisation already in this file are everything the new cases need
                              • grapharc/cli/graphrun.py:204-286 — styled run output: header, ADMITTED/REFUSED verdicts, tinted rejection rows — uncovered
                              • grapharc/cli/main.py:496-504 — styled trace rows (style.dim, style.cell, style.accent, style.err) — uncovered
                              • grapharc/cli/main.py:521-526 — styled metrics output — uncovered

                              Confirm the gap for yourself — run --check-only emits escapes on a tty that nothing asserts on:

                              uv run pytest tests/test_cli_style.py -q --collect-only # 4 styled ids: plan, models, models-check, demo-stage0printf'{"nodes": [{"name": "triage"}], "edges": [{"source": "__start__", "target": "triage"}, {"source": "triage", "target": "__end__"}]}\n'> /tmp/topo.json
                              script -qec "uv run python -m grapharc.cli.main run /tmp/topo.json --check-only" /dev/null | cat -v | grep '\^\['# ^[[38;5;… escapes are emitted on a tty; no test compares them against the piped form

                              What to change

                              1. Add hermetic cases to STYLED/COMPARABLE in tests/test_cli_style.py for the uncovered styled commands: run <topo> --check-only (admitted path), run <bad-topo> (REFUSED path, which styles key and value tints no current case reaches), trace <trace.jsonl>, and metrics <trace.jsonl> <run-id>. A fixture can write the topology file and produce the trace with one demo stage0 --trace run, the way test_viz_on_a_terminal_stays_pasteable_mermaid already does.
                              2. Keep the existing judgement calls: exit codes stay unpinned, and any command whose output varies run-to-run stays out of COMPARABLE (the run refused/admitted text is deterministic apart from the mkdtemp trace path, which the existing _TMPDIR normaliser already handles).
                              3. If parametrising requires per-case setup (a file argument), a small indirection — building the argv in the test from a tmp_path-scoped fixture — is fine; do not weaken the assertion (stripped tty output must equal piped stdout+stderr exactly).

                              Out of scope: serve (blocks on a socket), demo --model live runs and agent (need a real provider), replay/diff (their text is the engine formatter's, deliberately unstyled — see grapharc/cli/main.py and grapharc/cli/replay.py), and any change to grapharc/cli/style.py itself.

                              How to verify

                              uv run pytest -q
                              uv run ruff check .

                              The new parametrised cases in tests/test_cli_style.py are the proof. Check they can go red: add a stray style.ok(...) guarded by nothing (or an unconditional "\x1b[32m") to one line of the run verdict block in grapharc/cli/graphrun.py and the new run cases must fail while the existing four stay green — that is precisely the blind spot being closed.

                              Acceptance criteria

                              • run (admitted and refused), trace and metrics are covered by both the escapes-on-a-tty/none-in-a-pipe test and the strip-equality test
                              • The new cases are hermetic: no network, no credentials, no reliance on the host's terminal, and they pass on a machine with no provider configured
                              • Existing four cases and their ids are unchanged
                              • uv run pytest stays green and uv run ruff check . is clean
                              • Any README or cookbook sentence this changes is updated in the same pull request (several are byte-compared against real output by the test suite)

                              Skill level

                              good first issue — the pty harness, environment scrubbing and path normalisation are already written in the same file; the work is producing fixtures for four more argv lists and wiring them into the existing parametrisation. The one subtlety is keeping the cases deterministic (fixture files, not fresh mkdtemp paths, wherever the output embeds one) — copy how test_viz_on_a_terminal_stays_pasteable_mermaid obtains its trace. Questions welcome on the issue.

                              Activity

                              Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

                              Metadata

                              Metadata

                              Assignees

                              No one assigned

                                Labels

                                No labels
                                No labels

                                Type

                                No type

                                Projects

                                No projects

                                  Milestone

                                  No milestone

                                  Relationships

                                  None yet

                                  Development

                                  No branches or pull requests

                                  Issue actions