feat(skills): add artifact presenter
This commit is contained in:
@@ -93,6 +93,7 @@ Below you will find my preferences so that we can be more aligned as we work tog
|
||||
- Plan before we implement. Investigate the codebase, do research, and grill me with questions. I'd rather spend some time answering questions first and get a result I am happy with rather than something we'd have to do again.
|
||||
- Prototype or create PoCs for large features first. If we can identify if something will or will not work early, that will save time in the long run.
|
||||
- Do not touch production or live databases unless explicitly instructed to do so.
|
||||
- When presenting any single local file for my review, or a structured long-form report, plan, ADR, review, or summary, load the [`artifact-presenter`](../skills/document/artifact-presenter/SKILL.md) skill and open the complete file with mcat in its reusable Herdr pane instead of returning the full artifact inline.
|
||||
|
||||
## Subagents and Parallel work
|
||||
|
||||
|
||||
+2
-1
@@ -13,8 +13,9 @@
|
||||
"scripts": {
|
||||
"prepare": "effect-tsgo patch",
|
||||
"typecheck": "tsc --noEmit",
|
||||
"test": "npm run test:runner && npm run test:dictate-service && npm run test:links && node --test --experimental-strip-types extensions/dictate/index.test.ts extensions/firecrawl-search/index.test.ts extensions/shared/*.test.ts extensions/subagents/by-the-way.test.ts extensions/subagents/context-usage.test.ts extensions/subagents/manager.test.ts extensions/subagents/result-delivery.test.ts extensions/subagents/takeover.test.ts extensions/_workflows/*.test.ts extensions/pi-workflow/*.test.ts && vitest run extensions/file-search/index.spec.ts",
|
||||
"test": "npm run test:runner && npm run test:artifact-presenter && npm run test:dictate-service && npm run test:links && node --test --experimental-strip-types extensions/dictate/index.test.ts extensions/firecrawl-search/index.test.ts extensions/shared/*.test.ts extensions/subagents/by-the-way.test.ts extensions/subagents/context-usage.test.ts extensions/subagents/manager.test.ts extensions/subagents/result-delivery.test.ts extensions/subagents/takeover.test.ts extensions/_workflows/*.test.ts extensions/pi-workflow/*.test.ts && vitest run extensions/file-search/index.spec.ts",
|
||||
"test:runner": "python -m unittest discover -s scripts -p 'test_*.py'",
|
||||
"test:artifact-presenter": "python -m unittest discover -s skills/document/artifact-presenter/tests -p 'test_*.py'",
|
||||
"test:dictate-service": "PYTHONPATH=services/dictate/src python -m unittest discover -s services/dictate/tests -p 'test_*.py'",
|
||||
"test:links": "node --test scripts/pi-links.test.mjs",
|
||||
"test:live": "node --test --experimental-strip-types extensions/subagents/claude.test.ts extensions/subagents/codex.test.ts"
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
---
|
||||
name: artifact-presenter
|
||||
description: Present any single local file the user needs to review, including documents, source, archives, PDFs, images, and videos, through mcat in a reusable Herdr pane. Also use for structured long-form reports, plans, ADRs, reviews, and summaries unless the user requests inline chat.
|
||||
---
|
||||
|
||||
# Artifact presenter
|
||||
|
||||
Present reviewable files and substantial generated content in a reusable terminal document pane instead of duplicating the complete artifact in chat.
|
||||
|
||||
## Choose the artifact
|
||||
|
||||
Use this workflow in either case:
|
||||
|
||||
- The user needs to review one local file, regardless of its length or format.
|
||||
- The expected deliverable is a structured report, plan, ADR, review, or summary with several substantive sections or roughly 1,000 words.
|
||||
|
||||
Treat an explicit destination and format as authoritative. For generated content without a requested destination:
|
||||
|
||||
- Use `/tmp/pi-present/<descriptive-slug>.<extension>`.
|
||||
- Default structured prose to Markdown when no other format is implied.
|
||||
- Create `/tmp/pi-present/` when needed.
|
||||
- Add a timestamp only when replacing an earlier artifact would lose useful information.
|
||||
|
||||
Keep ordinary explanations, code diffs, logs, multi-file comparisons, and user-requested inline responses in chat. Present a file only when it is a user-facing review artifact, not merely because the agent read it during investigation.
|
||||
|
||||
Completion criterion: select exactly one existing or generated artifact path before presentation.
|
||||
|
||||
## Prepare generated content
|
||||
|
||||
Write the complete deliverable to the selected path. Keep it self-contained: the reader must not need omitted chat text to understand it.
|
||||
|
||||
Preserve repository documentation conventions. Creating a presentation artifact under `/tmp` does not authorize editing repository files when the request is otherwise read-only.
|
||||
|
||||
When presenting an existing file, pass the original file directly to mcat. Do not create a converted copy merely for display.
|
||||
|
||||
Completion criterion: the selected file exists and contains the complete review artifact.
|
||||
|
||||
## Present with mcat
|
||||
|
||||
Run the helper using its absolute path resolved from this skill directory:
|
||||
|
||||
```bash
|
||||
python3 <skill-directory>/scripts/present.py <artifact-path>
|
||||
```
|
||||
|
||||
The helper reuses the pane labeled `artifact presenter` in the current Herdr workspace. When none exists, it creates a right split without taking focus. It invokes `mcat <artifact-path>` directly so mcat can infer and display documents, source, archives, PDFs, images, and videos. It falls back to artifact-only delivery when Herdr or mcat is unavailable.
|
||||
|
||||
Completion criterion: capture whether the helper opened, reused, or skipped the viewer pane.
|
||||
|
||||
## Convert only when requested
|
||||
|
||||
Use mcat's conversion pipeline when the user requests a derivative artifact:
|
||||
|
||||
```bash
|
||||
mcat <input> --output md > <output.md>
|
||||
mcat <input> --output html > <output.html>
|
||||
mcat <input> --output image > <output.png>
|
||||
```
|
||||
|
||||
Use an explicit output path and preserve the source file. Present the resulting single file through the helper when the user needs to review it.
|
||||
|
||||
## Respond concisely
|
||||
|
||||
Return:
|
||||
|
||||
- a one- to three-sentence result summary;
|
||||
- the artifact path;
|
||||
- whether the reusable mcat pane was opened or reused, or why presentation fell back to the file only.
|
||||
|
||||
Do not repeat the complete artifact in chat. Honor a later request to paste any section inline.
|
||||
+195
@@ -0,0 +1,195 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Open one review artifact in a reusable right-side Herdr mcat pane."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Callable, Mapping, Sequence
|
||||
|
||||
PANE_LABEL = "artifact presenter"
|
||||
|
||||
RunCommand = Callable[..., subprocess.CompletedProcess[str]]
|
||||
FindExecutable = Callable[[str], str | None]
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class PresentationResult:
|
||||
status: str
|
||||
artifact: Path
|
||||
pane_id: str | None = None
|
||||
reason: str | None = None
|
||||
|
||||
def message(self) -> str:
|
||||
if self.status == "artifact-only":
|
||||
return f"artifact-only: {self.reason}; {self.artifact}"
|
||||
return f"{self.status}: pane {self.pane_id}; {self.artifact}"
|
||||
|
||||
|
||||
def run_json(
|
||||
executable: str,
|
||||
args: Sequence[str],
|
||||
run: RunCommand,
|
||||
) -> dict[str, object]:
|
||||
completed = run(
|
||||
[executable, *args],
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
value = json.loads(completed.stdout)
|
||||
if not isinstance(value, dict):
|
||||
raise ValueError("Herdr returned a non-object response")
|
||||
return value
|
||||
|
||||
|
||||
def panes_with_label(response: Mapping[str, object]) -> list[str]:
|
||||
result = response.get("result")
|
||||
if not isinstance(result, dict):
|
||||
return []
|
||||
panes = result.get("panes")
|
||||
if not isinstance(panes, list):
|
||||
return []
|
||||
pane_ids: list[str] = []
|
||||
for pane in panes:
|
||||
if not isinstance(pane, dict) or pane.get("label") != PANE_LABEL:
|
||||
continue
|
||||
pane_id = pane.get("pane_id")
|
||||
if isinstance(pane_id, str) and pane_id:
|
||||
pane_ids.append(pane_id)
|
||||
return pane_ids
|
||||
|
||||
|
||||
def created_pane_id(response: Mapping[str, object]) -> str:
|
||||
result = response.get("result")
|
||||
pane = result.get("pane") if isinstance(result, dict) else None
|
||||
pane_id = pane.get("pane_id") if isinstance(pane, dict) else None
|
||||
if not isinstance(pane_id, str) or not pane_id:
|
||||
raise ValueError("Herdr split response is missing result.pane.pane_id")
|
||||
return pane_id
|
||||
|
||||
|
||||
def run_mcat(
|
||||
herdr: str,
|
||||
mcat: str,
|
||||
pane_id: str,
|
||||
artifact: Path,
|
||||
run: RunCommand,
|
||||
) -> None:
|
||||
run(
|
||||
[herdr, "pane", "run", pane_id, mcat, str(artifact)],
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
|
||||
|
||||
def present_document(
|
||||
document: Path,
|
||||
*,
|
||||
env: Mapping[str, str] = os.environ,
|
||||
which: FindExecutable = shutil.which,
|
||||
run: RunCommand = subprocess.run,
|
||||
) -> PresentationResult:
|
||||
artifact = document.expanduser().resolve()
|
||||
if not artifact.is_file():
|
||||
raise ValueError(f"artifact is not a file: {artifact}")
|
||||
|
||||
herdr = which("herdr")
|
||||
mcat = which("mcat")
|
||||
workspace_id = env.get("HERDR_WORKSPACE_ID")
|
||||
source_pane_id = env.get("HERDR_PANE_ID")
|
||||
if not herdr:
|
||||
return PresentationResult("artifact-only", artifact, reason="herdr is unavailable")
|
||||
if not mcat:
|
||||
return PresentationResult("artifact-only", artifact, reason="mcat is unavailable")
|
||||
if not workspace_id or not source_pane_id:
|
||||
return PresentationResult("artifact-only", artifact, reason="not running in a Herdr pane")
|
||||
|
||||
try:
|
||||
pane_list = run_json(herdr, ["pane", "list", "--workspace", workspace_id], run)
|
||||
existing_panes = panes_with_label(pane_list)
|
||||
except (json.JSONDecodeError, OSError, subprocess.SubprocessError, ValueError) as error:
|
||||
return PresentationResult("artifact-only", artifact, reason=f"could not list Herdr panes: {error}")
|
||||
|
||||
for existing_pane in existing_panes:
|
||||
try:
|
||||
run_mcat(herdr, mcat, existing_pane, artifact, run)
|
||||
return PresentationResult("reused", artifact, pane_id=existing_pane)
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
try:
|
||||
run(
|
||||
[herdr, "pane", "rename", existing_pane, f"{PANE_LABEL} stale"],
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
try:
|
||||
split = run_json(
|
||||
herdr,
|
||||
[
|
||||
"pane",
|
||||
"split",
|
||||
"--pane",
|
||||
source_pane_id,
|
||||
"--direction",
|
||||
"right",
|
||||
"--cwd",
|
||||
str(artifact.parent),
|
||||
"--no-focus",
|
||||
],
|
||||
run,
|
||||
)
|
||||
pane_id = created_pane_id(split)
|
||||
except (json.JSONDecodeError, OSError, subprocess.SubprocessError, ValueError) as error:
|
||||
return PresentationResult("artifact-only", artifact, reason=f"could not create Herdr viewer: {error}")
|
||||
|
||||
try:
|
||||
run(
|
||||
[herdr, "pane", "rename", pane_id, PANE_LABEL],
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
run_mcat(herdr, mcat, pane_id, artifact, run)
|
||||
return PresentationResult("opened", artifact, pane_id=pane_id)
|
||||
except (OSError, subprocess.SubprocessError) as error:
|
||||
try:
|
||||
run(
|
||||
[herdr, "pane", "close", pane_id],
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
except OSError:
|
||||
pass
|
||||
return PresentationResult("artifact-only", artifact, reason=f"could not initialize Herdr viewer: {error}")
|
||||
|
||||
|
||||
def create_parser() -> argparse.ArgumentParser:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("artifact", type=Path, help="Local file to present with mcat")
|
||||
return parser
|
||||
|
||||
|
||||
def main() -> int:
|
||||
args = create_parser().parse_args()
|
||||
try:
|
||||
result = present_document(args.artifact)
|
||||
except ValueError as error:
|
||||
print(error, file=os.sys.stderr)
|
||||
return 2
|
||||
print(result.message())
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main())
|
||||
@@ -0,0 +1,228 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
MODULE_PATH = Path(__file__).parents[1] / "scripts" / "present.py"
|
||||
SPEC = importlib.util.spec_from_file_location("artifact_presenter", MODULE_PATH)
|
||||
assert SPEC and SPEC.loader
|
||||
MODULE = importlib.util.module_from_spec(SPEC)
|
||||
sys.modules[SPEC.name] = MODULE
|
||||
SPEC.loader.exec_module(MODULE)
|
||||
|
||||
|
||||
class FakeRunner:
|
||||
def __init__(
|
||||
self,
|
||||
panes: list[dict[str, str]] | None = None,
|
||||
*,
|
||||
failed_run_panes: set[str] | None = None,
|
||||
fail_rename: bool = False,
|
||||
) -> None:
|
||||
self.panes = panes or []
|
||||
self.failed_run_panes = failed_run_panes or set()
|
||||
self.fail_rename = fail_rename
|
||||
self.calls: list[list[str]] = []
|
||||
|
||||
def __call__(self, command, **_kwargs):
|
||||
argv = [str(value) for value in command]
|
||||
self.calls.append(argv)
|
||||
if argv[1:3] == ["pane", "run"] and argv[3] in self.failed_run_panes:
|
||||
raise subprocess.CalledProcessError(1, argv)
|
||||
if argv[1:3] == ["pane", "rename"] and self.fail_rename:
|
||||
raise subprocess.CalledProcessError(1, argv)
|
||||
if argv[1:3] == ["pane", "list"]:
|
||||
output = {"result": {"panes": self.panes}}
|
||||
elif argv[1:3] == ["pane", "split"]:
|
||||
output = {"result": {"pane": {"pane_id": "workspace:p-new"}}}
|
||||
else:
|
||||
output = {}
|
||||
return subprocess.CompletedProcess(argv, 0, json.dumps(output), "")
|
||||
|
||||
|
||||
class ArtifactPresenterTests(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.tempdir = tempfile.TemporaryDirectory()
|
||||
self.artifact = Path(self.tempdir.name) / "report.md"
|
||||
self.artifact.write_text("# Report\n", encoding="utf-8")
|
||||
self.env = {
|
||||
"HERDR_WORKSPACE_ID": "workspace",
|
||||
"HERDR_PANE_ID": "workspace:p-source",
|
||||
}
|
||||
self.executables = {"herdr": "/usr/bin/herdr", "mcat": "/usr/bin/mcat"}
|
||||
|
||||
def tearDown(self) -> None:
|
||||
self.tempdir.cleanup()
|
||||
|
||||
def which(self, name: str) -> str | None:
|
||||
return self.executables.get(name)
|
||||
|
||||
def test_reuses_named_pane(self) -> None:
|
||||
runner = FakeRunner(
|
||||
[{"pane_id": "workspace:p-viewer", "label": MODULE.PANE_LABEL}],
|
||||
)
|
||||
|
||||
result = MODULE.present_document(
|
||||
self.artifact,
|
||||
env=self.env,
|
||||
which=self.which,
|
||||
run=runner,
|
||||
)
|
||||
|
||||
self.assertEqual(result.status, "reused")
|
||||
self.assertEqual(result.pane_id, "workspace:p-viewer")
|
||||
self.assertIn(
|
||||
[
|
||||
"/usr/bin/herdr",
|
||||
"pane",
|
||||
"run",
|
||||
"workspace:p-viewer",
|
||||
"/usr/bin/mcat",
|
||||
str(self.artifact.resolve()),
|
||||
],
|
||||
runner.calls,
|
||||
)
|
||||
self.assertFalse(any(call[1:3] == ["pane", "split"] for call in runner.calls))
|
||||
|
||||
def test_tries_each_named_pane_before_creating_another(self) -> None:
|
||||
runner = FakeRunner(
|
||||
[
|
||||
{"pane_id": "workspace:p-stale", "label": MODULE.PANE_LABEL},
|
||||
{"pane_id": "workspace:p-viewer", "label": MODULE.PANE_LABEL},
|
||||
],
|
||||
failed_run_panes={"workspace:p-stale"},
|
||||
)
|
||||
|
||||
result = MODULE.present_document(
|
||||
self.artifact,
|
||||
env=self.env,
|
||||
which=self.which,
|
||||
run=runner,
|
||||
)
|
||||
|
||||
self.assertEqual(result.status, "reused")
|
||||
self.assertEqual(result.pane_id, "workspace:p-viewer")
|
||||
self.assertIn(
|
||||
[
|
||||
"/usr/bin/herdr",
|
||||
"pane",
|
||||
"rename",
|
||||
"workspace:p-stale",
|
||||
f"{MODULE.PANE_LABEL} stale",
|
||||
],
|
||||
runner.calls,
|
||||
)
|
||||
self.assertFalse(any(call[1:3] == ["pane", "split"] for call in runner.calls))
|
||||
|
||||
def test_presents_non_markdown_file_directly_with_mcat(self) -> None:
|
||||
video = self.artifact.with_name("review.mp4")
|
||||
video.write_bytes(b"video fixture")
|
||||
runner = FakeRunner(
|
||||
[{"pane_id": "workspace:p-viewer", "label": MODULE.PANE_LABEL}],
|
||||
)
|
||||
|
||||
result = MODULE.present_document(
|
||||
video,
|
||||
env=self.env,
|
||||
which=self.which,
|
||||
run=runner,
|
||||
)
|
||||
|
||||
self.assertEqual(result.status, "reused")
|
||||
self.assertIn(
|
||||
[
|
||||
"/usr/bin/herdr",
|
||||
"pane",
|
||||
"run",
|
||||
"workspace:p-viewer",
|
||||
"/usr/bin/mcat",
|
||||
str(video.resolve()),
|
||||
],
|
||||
runner.calls,
|
||||
)
|
||||
|
||||
def test_creates_right_pane_when_viewer_is_absent(self) -> None:
|
||||
runner = FakeRunner()
|
||||
|
||||
result = MODULE.present_document(
|
||||
self.artifact,
|
||||
env=self.env,
|
||||
which=self.which,
|
||||
run=runner,
|
||||
)
|
||||
|
||||
self.assertEqual(result.status, "opened")
|
||||
split = next(call for call in runner.calls if call[1:3] == ["pane", "split"])
|
||||
self.assertEqual(
|
||||
split,
|
||||
[
|
||||
"/usr/bin/herdr",
|
||||
"pane",
|
||||
"split",
|
||||
"--pane",
|
||||
"workspace:p-source",
|
||||
"--direction",
|
||||
"right",
|
||||
"--cwd",
|
||||
str(self.artifact.parent.resolve()),
|
||||
"--no-focus",
|
||||
],
|
||||
)
|
||||
self.assertIn(
|
||||
[
|
||||
"/usr/bin/herdr",
|
||||
"pane",
|
||||
"rename",
|
||||
"workspace:p-new",
|
||||
MODULE.PANE_LABEL,
|
||||
],
|
||||
runner.calls,
|
||||
)
|
||||
|
||||
def test_closes_new_pane_when_rename_fails(self) -> None:
|
||||
runner = FakeRunner(fail_rename=True)
|
||||
|
||||
result = MODULE.present_document(
|
||||
self.artifact,
|
||||
env=self.env,
|
||||
which=self.which,
|
||||
run=runner,
|
||||
)
|
||||
|
||||
self.assertEqual(result.status, "artifact-only")
|
||||
self.assertIn("could not initialize Herdr viewer", result.reason)
|
||||
self.assertIn(
|
||||
["/usr/bin/herdr", "pane", "close", "workspace:p-new"],
|
||||
runner.calls,
|
||||
)
|
||||
|
||||
def test_falls_back_to_artifact_when_mcat_is_unavailable(self) -> None:
|
||||
self.executables.pop("mcat")
|
||||
|
||||
result = MODULE.present_document(
|
||||
self.artifact,
|
||||
env=self.env,
|
||||
which=self.which,
|
||||
run=FakeRunner(),
|
||||
)
|
||||
|
||||
self.assertEqual(result.status, "artifact-only")
|
||||
self.assertEqual(result.reason, "mcat is unavailable")
|
||||
|
||||
def test_rejects_missing_artifact(self) -> None:
|
||||
with self.assertRaisesRegex(ValueError, "artifact is not a file"):
|
||||
MODULE.present_document(
|
||||
self.artifact.with_name("missing.md"),
|
||||
env=self.env,
|
||||
which=self.which,
|
||||
run=FakeRunner(),
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user