Skip to content

feat(cli): --selector flag scoping lint to subtree - #127

Merged
aram-devdocs merged 3 commits into
mainfrom
feat/cli-selector-flag-32
Apr 25, 2026
Merged

feat(cli): --selector flag scoping lint to subtree#127
aram-devdocs merged 3 commits into
mainfrom
feat/cli-selector-flag-32

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Summary

Closes #32. Adds --selector <css-selector> to plumb lint, scoping the run to elements matching the selector and their descendants. Filtering happens between snapshot collection and rule dispatch, so rules see exactly the subtree the user asked for.

  • New flag on plumb lint. Optional, single value. When omitted, behavior is unchanged.
  • New module crates/plumb-cli/src/commands/selector.rs round-trips snapshot → tagged HTML → scraper::Selector matcher → snapshot. Match-and-descendants kept; parent/children references rewritten consistently. Sorted-deduped match set + BTreeSet for the kept set keep output byte-stable.
  • scraper added to plumb-cli only (workspace layer rule). Built with default-features = false + errors + deterministic.
  • Errors map to exit 2 (CLI / infra failure per PRD §13.3): invalid selector → invalid --selector \`: ; zero matches → --selector `` matched no elements in the snapshot`.

Test plan

  • cargo nextest run -p plumb-cli — 12 unit tests in selector.rs + 4 new CLI integration tests, all green.
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — clean.
  • cargo fmt --all -- --check — clean.
  • just determinism-check — three runs byte-identical.
  • just validate — all gates pass.

Acceptance criteria

  • CSS selector supported via scraper's matcher.
  • Selector filtering applied before rule dispatch.
  • CLI integration test (4 added: body match keeps violation, head-only drops violation, invalid selector exits 2, no-match exits 2).

🤖 Generated with Claude Code

aram-devdocs and others added 3 commits April 25, 2026 16:39
Adds the `--selector` building block: a `filter_snapshot` that maps a
`PlumbSnapshot` through a `scraper::Selector` round-trip and returns
only the matched subtrees. Lives in `plumb-cli` to keep CSS-selector
parsing out of the deterministic core layer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the `--selector <CSS_SELECTOR>` flag to `plumb lint`. When set,
each viewport's snapshot is filtered through `selector::filter_snapshot`
between collection and rule dispatch, so rules see exactly the subtree
the user asked to lint. Invalid selectors and zero-match selectors both
surface as exit code 2 (CLI / infrastructure failure, PRD §13.3).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four end-to-end cases against `plumb-fake://hello`: matching `body`
keeps the canned `spacing/grid-conformance` warning (exit 3); matching
only `head` drops it (exit 0); an unparseable selector exits 2 with
`invalid --selector` on stderr; a parseable selector that matches
nothing exits 2 with `matched no elements` on stderr.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@aram-devdocs
aram-devdocs force-pushed the feat/cli-selector-flag-32 branch from 7def654 to f095bd7 Compare April 25, 2026 22:40
@github-actions

Copy link
Copy Markdown
Contributor

I now have everything I need. Here is the review:


PR Review: feat/cli-selector-flag-32--selector filter for plumb lint

Files in scope: plumb-cli only (correct layer).


1. Determinism

crates/plumb-cli/src/commands/selector.rs:566–566
The module doc explicitly lists the determinism guarantees: BTreeSet for the kept set, sort_unstable + dedup on matched, IndexMap for the index. The scraper dep is pinned with features = ["deterministic"], which forces a fixed-seed hasher inside selectors/cssparser rather than the randomly-seeded default. The deliberate determinism unit test at line 943 closes the loop. No wall-clock, no HashMap in any observable path. Clean.


2. Workspace layering

All new code lands in plumb-cli. The scraper dep is added only to plumb-cli/Cargo.toml. plumb-core and plumb-format are untouched. deny.toml already allowlists MPL-2.0, which covers cssparser (the deepest transitive dep with that license). Clean.


3. Error handling

SelectorError derives thiserror::Error. No unwrap/expect outside the #[cfg(test)] module. The conversion to anyhow::Error in lint.rs:517 via .map_err(anyhow::Error::from) is the correct pattern for the CLI boundary. The # Errors section on filter_snapshot is present. Clean.


4. Punch list

Warning — selector.rs:739–780 — recursive write_node serializer

fn write_node(out, snapshot, index, node) {
    ...
    for &child_order in &node.children {
        write_node(out, snapshot, index, &snapshot.nodes[i]);  // recursive
    }
}

Frame footprint is small (~100–120 bytes) and Rust's default 8 MB stack allows ~70 K levels, which comfortably exceeds any browser-enforced DOM depth limit. The risk is theoretical but the iterative version (explicit Vec<&SnapshotNode> stack) is equally simple and eliminates the reasoning burden entirely. Not a blocker today; worth a follow-up issue.

Notice — selector.rs:629–630 — HTML5 structural normalization by html5ever

let html = serialize(&snapshot);
let document = scraper::Html::parse_document(&html);

html5ever auto-inserts <tbody> between <table> and <tr>, so the CSS selector table > tr will silently match nothing even if the snapshot contains that structure directly. The structural selector table > tbody > tr would be required. This is correct HTML5 parsing behaviour, but it is a user-visible gotcha for table-heavy pages. Consider a one-line note in the --selector help text or the SelectorError::NoMatch message.

Notice — selector.rs:747node.tag emitted verbatim

out.push_str(&node.tag);

SnapshotNode::tag is documented as lowercase and comes from CDP, so in practice it is always a valid HTML element name. Still, the serializer should be robust: a tag containing > or " (impossible from CDP but possible in unit tests) would corrupt the output document and silently produce wrong selector matches rather than an error. Low practical risk.


5. Test coverage

Unit tests at lines 852–975 cover: descendants kept, parent cleared, siblings dropped, grouped selector, no-match error, parse error, universal selector, canned snapshot (positive + negative), determinism, attribute round-trip. Integration tests at lines 1027–1073 cover all four CLI paths against the real binary. Complete.


Verdict: APPROVE

@aram-devdocs
aram-devdocs merged commit f61428b into main Apr 25, 2026
14 checks passed
@aram-devdocs
aram-devdocs deleted the feat/cli-selector-flag-32 branch April 25, 2026 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cli): --selector flag scoping lint to subtree

1 participant