Enable tracing artifacts and pretty result dumps #61
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enables enhanced tracing functionality by adding artifact support and pretty-printed result dumps. The main purpose is to improve debugging and inspection capabilities for POML executions by generating additional trace files with human-readable outputs.
Key changes include:
- Adding
trace_artifactfunction to Python API for creating custom trace files - Dumping pretty-printed results to
.result.txtfiles in JavaScript traces - Implementing regex-based logic to find the latest trace prefix instead of tracking globals
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| python/poml/init.py | Exports new trace_artifact function |
| python/poml/api.py | Implements trace_artifact and _latest_trace_prefix helper functions |
| python/tests/test_basic.py | Adds comprehensive tests for artifact functionality and prefix regex logic |
| packages/poml/util/trace.ts | Adds support for dumping pretty-printed results to .result.txt files |
| packages/poml/index.ts | Refactors output generation and passes pretty output to trace dump |
| packages/poml/tests/trace.test.tsx | Tests that pretty-printed result text is properly dumped |
| if not (_trace_enabled and _trace_dir): | ||
| return None | ||
|
|
||
| pattern = re.compile(r"^(\d{4}.*?)(?:\.source)?\.poml$") |
There was a problem hiding this comment.
The regex pattern will incorrectly match files ending with '.source.poml' and treat them as valid trace files. The negative lookahead should be modified to properly exclude source files while still matching the main trace files.
| pattern = re.compile(r"^(\d{4}.*?)(?:\.source)?\.poml$") | |
| pattern = re.compile(r"^(\d{4}.*?)(?!\.source\.poml)\.poml$") |
| if prefix_part.endswith(".source"): | ||
| continue |
There was a problem hiding this comment.
This check for '.source' suffix is redundant and potentially incorrect since the regex already handles this case. The logic should be consistent between the regex pattern and this manual check.
| if prefix_part.endswith(".source"): | |
| continue |
| def trace_artifact(file_suffix: str, contents: str | bytes) -> Optional[Path]: | ||
| """Write an additional artifact file for the most recent ``poml`` call.""" | ||
| prefix = _latest_trace_prefix() | ||
| if prefix is None: | ||
| return None | ||
| suffix = file_suffix if file_suffix.startswith(".") else f".{file_suffix}" |
There was a problem hiding this comment.
[nitpick] Consider extracting this suffix normalization logic into a helper function or using a more explicit conditional to improve readability and potential reuse.
| def trace_artifact(file_suffix: str, contents: str | bytes) -> Optional[Path]: | |
| """Write an additional artifact file for the most recent ``poml`` call.""" | |
| prefix = _latest_trace_prefix() | |
| if prefix is None: | |
| return None | |
| suffix = file_suffix if file_suffix.startswith(".") else f".{file_suffix}" | |
| def _normalize_suffix(file_suffix: str) -> str: | |
| """Ensure the file suffix starts with a dot.""" | |
| return file_suffix if file_suffix.startswith(".") else f".{file_suffix}" | |
| def trace_artifact(file_suffix: str, contents: str | bytes) -> Optional[Path]: | |
| """Write an additional artifact file for the most recent ``poml`` call.""" | |
| prefix = _latest_trace_prefix() | |
| if prefix is None: | |
| return None | |
| suffix = _normalize_suffix(file_suffix) |
Summary
.result.txtfor JS tracestrace_artifacthelper to Python APITesting
npm run build-webviewnpm run build-clinpm run lintnpm testpython -m pytest python/testshttps://chatgpt.com/codex/tasks/task_e_68875447fe1c832e922bc6e3fcb1ee58