Repository navigation
feat(cli): add timeline --format md - #182
Conversation
Renders the filtered timeline as a Markdown table (Date, Type, Event, Files) suitable for piping into reports and standup notes. Pipes and line breaks inside a message are escaped so it cannot break the table; an empty log emits an italic placeholder. The default terminal output is untouched when --format is absent. Resolves mex-memory#55
798aca7 to
8e3fadc
Compare
|
Rebase note:
All 30 events tests and the abstention golden pass locally; CI green on the rebased head (check 22.22.0/24, hub-browser, release-performance, storage-portability macos+windows). Ready for review. |
theDakshJaitly
left a comment
There was a problem hiding this comment.
Requesting changes for the four inline findings. The CLI currently drops the new format option, so mex timeline --format md still prints ordinary terminal output. The Markdown renderer also needs to preserve omission notices, correctly encode table cells, and enforce the output limit after rendering.
Validation: applied the exact PR diff to main in an isolated checkout; 53 existing events/public API tests, typecheck, build, and diff checks passed. Targeted probes reproduced the four issues. CLI forwarding was also checked on the PR head.
| .command("timeline") | ||
| .description("Read bounded recent project notes (latest 8 MiB / 10,000 log lines)") | ||
| .option("--json", "Output events as JSON") | ||
| .option("--format <format>", "Output format: md for a Markdown table (piped into reports)") |
There was a problem hiding this comment.
[P1] Forward the format option to the timeline handler
The action below builds a new options object for runTimeline but never includes format: opts.format. As a result, the built CLI accepts mex timeline --format md and exits successfully while still printing ordinary terminal text; the new renderer is unreachable through the advertised command. Please forward the option and add a test invoking the actual CLI, since the new tests call runTimeline directly and bypass this dispatch.
There was a problem hiding this comment.
Fixed in 7de394c: the CLI now forwards format. The built-CLI regression checks Markdown dispatch, default output, and JSON precedence. Child processes and the outer test are bounded in a03a4ba/f08fbd1; the exact test passes locally. Final-head CI is green: https://github.com/mex-memory/mex/actions/runs/35165138729
| if (selected.length === 0) { | ||
| console.log("_No events found._"); | ||
| return; |
There was a problem hiding this comment.
[P2] Preserve omission notices when the selected result is empty
This early return skips both truncated and sourceTruncated notices. A matching event with a 70,000-character message is excluded by the output budget and produces only _No events found._; likewise, a query matching only an older entry outside the retained 10,000-line window hides the incomplete source scan. Please emit the applicable omission notices even when no rows are selected, and distinguish an omitted matching event from a genuinely empty result.
There was a problem hiding this comment.
Fixed in 7de394c. Empty selection now retains both omission notices and distinguishes budget-omitted matches from no matches. Added regressions for oversized entries and matches outside the retained history window.
| const message = e.message.replace(/\|/g, "\\|").replace(/\r?\n/g, " "); | ||
| const files = e.files.length ? e.files.map((f) => `\`${f}\``).join(", ") : "—"; |
There was a problem hiding this comment.
[P2] Encode messages and file paths as Markdown table cells
File paths are inserted verbatim inside single backticks, so accepted paths containing a pipe (such as src/a|b.ts), newline, or backtick break the table or code span. The message escaping also fails when a pipe already has a backslash before it, and leaves bare carriage returns intact. These cases reproduce with the renderer and a Markdown table parser. Please use table-aware encoding for both messages and paths, handling backslashes, CR/LF, and code-span delimiters, and test the rendered cell contents.
There was a problem hiding this comment.
Fixed in 7de394c. Messages and paths handle pipes, backslashes, CR/LF, and backtick runs. Regression tests parse the output with GFM and verify four cells and round-tripped content.
| for (const e of selected) { | ||
| const message = e.message.replace(/\|/g, "\\|").replace(/\r?\n/g, " "); | ||
| const files = e.files.length ? e.files.map((f) => `\`${f}\``).join(", ") : "—"; | ||
| console.log(`| ${e.timestamp.slice(0, 10)} | ${e.kind} | ${message} | ${files} |`); |
There was a problem hiding this comment.
[P2] Apply the byte limit to the rendered Markdown
The shared selection budget measures JSON serialization, but Markdown escaping can make the emitted rows substantially larger. A single accepted message containing 40,000 pipe characters produces 80,081 UTF-8 bytes here, exceeding the 65,536-byte output limit. Please budget the actual escaped rows together with the table header and omission notices before emitting them, and report when a complete row cannot fit.
There was a problem hiding this comment.
Fixed in 7de394c. Markdown rows are budgeted after escaping, including UTF-8 bytes, headers, newlines, and notices; oversized rows are omitted whole. All 48 events tests pass, including the byte-boundary cases. Final-head CI is green; @theDakshJaitly could you re-review?
|
hi all four review points are fixed and pushed the cli forwards the format option so timeline with format md actually renders markdown empty selections keep both omission notices and budget omitted results are separate from no matches messages and file paths are encoded as proper table cells handling pipes backslashes newlines and backtick runs the byte limit applies to the rendered markdown and oversized rows are dropped whole the events suite passes fully and final head ci is green could you re review when you get time |
theDakshJaitly
left a comment
There was a problem hiding this comment.
Re-reviewed the updated commits against all four requested changes. CLI format forwarding, omission notices for empty selections, GFM cell encoding, and the rendered 64 KiB output bound are now fixed. No remaining findings.
Verified the complete PR patch on current main: 91 focused tests passed, including the original review probes, the built-CLI output regression, public API coverage, and CLI smoke tests. Build, workspace typecheck, and diff checks passed; default and JSON output remain unchanged. CI is green on this head.
Resolves #55.
What
mex timeline --format mdrenders the (already-filterable —--since,--type,--limitall compose) timeline as a Markdown table:\|, line breaks → spaces) so a message cannot break the table; file paths render as inline code._No events found._— valid Markdown, still readable when pasted.--jsontakes precedence (unchanged); the default terminal output is byte-identical when--formatis absent.Tests
Three new cases in
test/events.test.ts: table shape + escaping with a pipe-bearing message and a file column, empty-log placeholder, and default output unchanged.npm run typecheckgreen.