[tonyalaribe]

Infrastructure inventories now render the same Pkg.Components.Facets.sidebar_ as Explorer. The shared renderer handles counted checkboxes, grouped fields, search, lazy values, and More/Less controls, and is embedded by the table and Host Map.

The table owns optional export, column customization, grouping, and resource tabs. Column choices persist through refresh and HTMX navigation; CSS owns visibility and inline hyperscript restores preferences. Host grouping works with search and CSV export. Switching Kubernetes tabs preserves filters and time, applying namespace scope before aggregation so the node and cluster views keep matching resources.

Container identity moves into the name column at narrower desktop widths so CPU values remain visible alongside the shared sidebar. The obsolete Host Map time-input helper is removed. Two existing browser tests now await editor readiness and saved review settings. AGENTS.md reserves expensive sign-off for the end of implementation and verification.

Validation:

• Repeated Haskell distillation, constraint, locality, and final simplification reviews; applied findings.
• All 160 browser tests passed with one worker.
• All 1,856 doctests and 1,029 frontend tests passed.
• Build, unit tests, lint, formatting, and dead-code checks passed.
• Focused infrastructure integration coverage: 19 examples passed.
• Full integration suite: 1,201 examples, zero failures (two pending), with four shards.
• Production image built and pushed for the PR commit.
• E2E_WORKERS=1 CI_SHARDS=4 make ci-signoff passed; every check has a valid attestation for this tree.

Review decisions:

• Supported pages each render one facet rail. Scoping search for multiple simultaneous rails is deferred until such a page exists.

• Keep facetTail_'s expanded flag: inventory overflow passes False, while Explorer's lazy response passes True (Log.hs:1002). Dropping it would change Explorer behavior.

• Namespace-before-aggregation is already exercised through real requests in infrastructure-time-window.spec.ts:86: Pods → Nodes retains the namespace/cluster, returns both matching rows, and preserves hidden columns.

• Keep the qualified Facets.Config / Table.Config names and explicit configuration fields. Attribute bags would weaken the API. Changing infraUrl to optional values would add conversions to concrete-value callers without removing URL handling.

• The two Kubernetes aggregations consume different inputs: the full snapshot supplies facet options/counts, and namespace-scoped rows supply the table. Counts intentionally retain the unfiltered inventory scope, consistent with the other inventories; a local alias would not remove either computation.

• Keep presentation sorting at the host table boundary, rather than coupling shared inventory projection to table order. Keep the small positional filter-option constructor consistent with its existing builders.

• Applied the review’s exhaustive constructor classification: one explicit case covers values, children, and loading, and its result drives both inline style decisions. The small More-label expressions remain beside their markup rather than adding another helper.

• Final automated review: keep optional clear/actions values because absence omits their wrappers; empty attributes alone would still render a Clear button without behavior. Keep lazy-tail expansion separate from the fetch/expand strategy because an Explorer response opens a fetched tail while inventories initially collapse it. A values-only predicate would lose the explicit constructor coverage required by this repository.

• The Explorer checkbox's short hx-on:click interop and unloaded-group search behavior predate this extraction. Selectors passed to filterInputAttr_ are static code literals. Additional inventory defaults, URL builders, vector-sorting dependencies, and layout-only helpers would widen this change without fixing observed behavior, so they are deferred.

• Browser coverage ran with E2E_BASE_URL set: grouping/search/CSV and namespace tab navigation were exercised. The CSV assertion pins the current export format, and explicit viewport/breakpoint expectations test the responsive contract; no duplicate handler test or breakpoint configuration layer is added.

[monoscope-bot]

Monoscope production impact · Coverage unknown

Project 87576849-4941-49d3-a15d-680fef88a1a8 · reviewed 6a474606844c9c9635d7e35516dce2ec54663bf4 · advisory

Evidence window (UTC): 2026-10-19 00:27:28.202216734 UTC → 2026-10-26 00:27:28.202216734 UTC

Coverage: Evidence is capped at 20 service/environment groups, monitors and dashboard queries. Telemetry describes the mapped service, not a proven changed-path or deployed-revision match. Monitor and dashboard references are project-wide; their relationship to the mapped service is unverified. Diff coverage is incomplete: only the first 30 files and 2000 characters per patch are inspected. No evidence maps the changed revision to a deployed service, environment, or changed request path. No changed-path performance or export/facet interaction measurements were supplied. Diff inspection is incomplete beyond the first 30 files and 2000 characters per patch. The supplied monitor and dashboard references lack verified dependency or revision linkage to the changed paths.

Review history and settings

[claude]

Review

Good consolidation: it removes the bespoke singleSelectFilter, mapSelect, HostListRow and hostGroupControl code and puts one facet renderer behind Explorer, the tables and Host Map. I could only read the first ~1200 lines of the diff, so Table.hs past countedFilter, Log.hs, Components.hs and Containers.hs are not covered.

Possible bug

• Facets.hs sidebar_: the search input uses the global selector ".facet-rail .facet-item, .facet-rail .facet-section". Any page with two rails filters both from one box. Scope it to the closest rail.

Succinctness / reuse

• facetNode_ does case node.content of twice, once with FacetValues{} and once with _. Compute it once. The _ -> wildcard also breaks the exhaustive-case rule in AGENTS.md; spell out FacetChildren/FacetLoading.
• The "+ More (n)" label is built twice, in values_ (moreLabel) and in facetTail_. Share one helper.
• facetTail_ :: Bool -> … takes a boolean flag, and ExpandValues always passes False. Drop the param.
• Facets.Config and Table.Config share a name. elemId and extraClass could be a single [Attribute].
• mapMaybe sequenceA [...] is repeated in the Kubernetes tabs and mapUrl. infraUrl takes [(Text, Text)] while deferUrl takes [(Text, Maybe Text)]. Let infraUrl take Maybe values and drop the call-site mapMaybe sequenceA.
• Kubernetes handler: kubeRowsFromRows resource runs twice. A local build = kubeRowsFromRows resource is tidier. Check that cluster/status facet counts are meant to ignore the namespace scope.
• Hosts: V.fromList $ sortOn … $ V.toList hosts round-trips through a list. Sorting before the Vector is built avoids the copy (minor).
• multiSelectFilter builds FilterOption positionally with a trailing Nothing. Record syntax is more robust.

Tests

• e2e coverage looks good. The countedFilter doctest pins counts and the empty-value drop. Add a test for the Kubernetes namespace-before-aggregation change in kubernetesGetH, since it changes behavior.

Security/perf

• No concerns. Values go through toHtml and data-tippy-content, which are escaped. Counting is O(n) per facet.

[claude]

Review (skimmed the diff; build/CI results taken from the PR description).

Good: one shared Facets.sidebar_ across Explorer, tables and Host Map. Filter counts are computed before filtering, and the countedFilter doctest pins that. The Kubernetes tab swap keeps namespace scoping, and e2e covers it.

Succinctness / reuse

• Facets.hs: sidebar_'s rail branches on cfg.layout twice, once for the wrapper and once for the class suffix. Fold the max-lg:w-full suffix into the Collapsible arm, or move layout into extraClass.
• Facets.hs: Config.clear :: Maybe [Attribute] and FacetNode.actions :: Maybe (Html ()) are only ever used with forM_. [Attribute] and Html () are already empty-able, so mempty could replace the Maybe. FacetOverflow and facetTail_'s expanded :: Bool are two encodings of one idea. You noted you're keeping the flag; at least derive it from FacetOverflow so the invariant lives in one place.
• Facets.hs: grouped in facetNode_ is a hand-written 3-arm case over FacetContent. A FacetValues{}-only predicate (isValues) would be shorter. The facetGroupNode lazy-attrs if loaded then [] else [...] could be [a | not loaded, a <- ...].
• Infrastructure.hs and Containers.hs: each page repeats config = def{elemID, containerId, addPadding = True, renderAsTable = True} plus customizeColumns = True. Make renderAsTable/addPadding/customizeColumns defaults for inventory tables, or add a inventoryConfig elem container helper.
• Infrastructure.hs: the provider/region/os countedFilter triples appear in both the hosts table and the Host Map. The URL-building mapMaybe sequenceA [...] is also repeated 3+ times (hosts, kubernetes, map). A single filtersToParams :: HostFilters -> [(Text,Text)] would remove the duplication.
• Infrastructure.hs: sortOn (\h -> ...) $ V.toList hosts then V.fromList. Use V.modify with Data.Vector.Algorithms or sort once upstream. hostGroupLabel is called per comparison and again for rowGroup, so precompute it with sortOn (which caches keys only on a decorated list).
• Log.hs: the facetChoice Aria.label_/data-tippy-content/data-field/data-value set is stringly. Fine, but hx-on:click raw JS (window.queryEditorCall(...)) goes against the hyperscript-first rule in AGENTS.md. A hyperscript on click call window.queryEditorCall('toggleSubQuery', my @data-field, my @data-value) is shorter and consistent.
• Components.hs: filterInputAttr_ embeds a long hyperscript string in a [text|..|] block with ${sel} interpolation. Make sure sel is only ever a static selector. It's injected into <${sel}/>, so any user-derived value there would be an XSS vector.

Bugs / risks

• facetNode_ renders a FacetLoading node with hx-trigger "toggle[target.open] once", and the sidebar search filters by .facet-item/.facet-section. Lazy, unloaded groups therefore can't match a search. Worth a note in the UI ("search covers loaded groups only") or a test.
• The e2e CSV test asserts csv.split('\n')).toHaveLength(3), which breaks if a trailing newline is ever added. Use a trimmed split.
• The Playwright tests are skipped when E2E_BASE_URL is unset, so there is no integration coverage of the grouped export. The PR says 19 focused infra examples pass, so a handler-level assert on group= plus the search case would be cheap.
• e2e/tests/infrastructure-time-window.spec.ts: width-dependent expectations (width < 1536, 390/1440) hard-code Tailwind breakpoints. Import them or comment them.

Security: nothing blocking. Facet values go through toHtml and term attributes, so they are escaped. Only the filterInputAttr_ selector interpolation above needs a static-input guarantee.

Process: the AGENTS.md sign-off wording is fine, though it is long for a one-rule change.