iter-141: output hygiene — text padding, invalid JSON, snapshot economics - #180
Merged
Conversation
…g, surface truncation Dogfooding session 63 found several output-hygiene defects: --format text padded every table row to the width of the single longest cell (255KB for 39 console rows); index concatenated a navigate envelope per crawled page ahead of its own summary JSON, breaking `| jq`; its robots.txt parser ignored User-agent grouping, applying a foreign bot's `Disallow: /` to every crawler; snapshot's truncation flag was buried inside results with no meta signal; empty --format text results printed a bare `[]` that hid sampled/capped context; JS exceptions during dom/click/wait bypassed the JSON error envelope entirely; jq syntax errors were misclassified as Internal instead of User; and network's slowest-20 list had no truncation marker. - output_pipeline.rs: middle_ellipsis now bounds every table cell, not just `url` columns; empty result arrays render sampled/capped context instead of a bare `[]`. - index.rs: crawl_page now calls navigate::run_core (non-printing) instead of the printing navigate::run, so stdout is exactly one JSON document; robots.txt is now parsed into User-agent groups and only the wildcard group's Disallow rules apply; outgoing links are deduped against an `enqueued` set to stop redundant queue growth. - snapshot.rs: meta now reports `truncated`/`text_truncated` explicitly; --max-chars help text describes the actual (whole-tree) bound. - js_helpers.rs: eval_or_bail/poll_js_condition route JS exceptions through AppError::User (the JSON envelope) instead of printing to stderr and exiting silently. - output_pipeline.rs: finalize_with_hints now returns AppError directly so jq parse/compile/runtime errors and --jq-strict missing-path errors classify as error_type "User", not "Internal". - network.rs: build_network_summary adds an explicit `slowest_truncated` flag (JSON and --format text) for the silent 20-request cap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…envelope Review of iter-141 (output hygiene / Theme E) found eval.rs's own inline exception handling still bypassed the JSON envelope — printing `error: ...` to stderr and returning AppError::Exit(1), the exact same anti-pattern Theme E fixed in eval_or_bail/poll_js_condition (js_helpers.rs). `eval` is the most direct way to trigger a raw JS exception, so this closes the last gap: AppError::User now carries the exception message through main's single JSON-envelope emission, consistent with every other command failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All 8 acceptance criteria landed in this PR (verified against the diff by ac-fidelity-check.sh); annotate the heading per the iter-137 convention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…velope-bypass bug iter-141's review pass changed eval.rs's inline JS-exception handler to route through AppError::User (JSON envelope) instead of bare stderr + Exit(1). Flag this for whoever implements iter-142 Theme E (which also touches eval.rs's wrapper), and note the same envelope-bypass shape still exists in click.rs/scroll.rs — worth a look if Theme E has runway, otherwise a follow-up plan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--format texttable rendering now middle-ellipsizes every cell (not justurlcolumns) — fixes the 255 KB / 8725-column blowup a single longconsolemessage caused.indexnow callsnavigate::run_core(non-printing) instead of the printingnavigate::run, so stdout is exactly one JSON document; itsrobots.txtparser now respectsUser-agent:grouping (a foreign agent'sDisallow: /no longer blocks a generic crawl); outgoing links are deduped against anenqueuedset.snapshot'smetanow reportstruncated/text_truncatedexplicitly instead of a marker buried insideresults;--max-charshelp text now describes the actual (whole-tree) bound.--format textresult array now surfacessampled/cappedcontext instead of printing a bare[].dom/click/wait/etc. (e.g. an invalid CSS selector) are now routed through the standard{"error":…,"error_type":"User"}JSON envelope instead of a bareerror: ...line on stderr with no JSON at all.OutputPipeline::finalize_with_hintsnow returnsAppErrordirectly so jq syntax/compile/runtime errors and--jq-strictmissing-path errors classify aserror_type: "User", not"Internal";network'sslowest(top-20) list now carries an explicitslowest_truncatedmarker.sources'actor: "",--fieldssilently dropping unknown names,cookies/storagetext-mode formatting polish,network --format textvs--detailtotal reconciliation — see the iteration plan's "Theme F disposition" note.Test plan
cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace -q(860+ unit/e2e tests green)cargo run -p xtask -- check-iteration-ready --plan kb/iterations/iteration-141-output-hygiene.md --base origin/main— 10/10 PASSoutput_pipeline.rs(cell-bounding, empty-results),index.rs(robots.txt UA grouping, enqueue dedup),snapshot.rs(snapshot_truncation_flags),network.rs(slowest_truncated)e2e_invalid_selector_json_envelope,e2e_network_truncation_flag,e2e_jq_error_type_is_user,e2e_jq_strict_missing_path_error_type_is_userFF_RDP_LIVE_TESTS=1, daemon-mode default per iter-137 discipline):live_141_console_text_bounded,live_141_index_single_json_document,live_141_index_robots_user_agent_groups,live_141_snapshot_truncation_in_meta,live_141_text_empty_result_keeps_metadata🤖 Generated with Claude Code## Claims vs code
<generated 2026-08-11T11:53:31Z by ralph-loop>
an→ ✅ matched in diffAppError::User→ ✅ matched in diffReview (review-pr, local-only)Local diff review (no Copilot — invoked with
local-only). Found one issue:eval's own inline JS-exception handler still printederror: ...to stderr and returnedAppError::Exit(1), bypassing the JSON error envelope entirely — the exact same anti-pattern Theme E fixed ineval_or_bail/poll_js_condition, just not applied toeval.rsitself.Fixed: routed through
Err(AppError::User(...)), matching the pattern already established elsewhere in this PR; updatedeval_exception_exits_nonzeroto assert on the stdout JSON envelope instead of stderr text. All quality gates (fmt, clippy -D warnings, cargo test --workspace, check-iteration-ready 10/10 with FF_RDP_LIVE_TESTS=1) re-verified green after the fix.Noted but not fixed (out of scope, flagged in iteration-142's plan instead): the same envelope-bypass shape still exists in
click.rs(2 call sites) andscroll.rs'srun_untiltimeout — no live-test coverage for those paths currently, and click.rs documents the stderr/stdout divergence as deliberate in an existing comment, so it needs its own verification pass rather than a rushed sweep here.