diff --git a/.github/instructions/unittestMcp.instructions.md b/.github/instructions/unittestMcp.instructions.md index 3a715a9..ed2ffce 100644 --- a/.github/instructions/unittestMcp.instructions.md +++ b/.github/instructions/unittestMcp.instructions.md @@ -33,11 +33,12 @@ When the user asks for tests on **multiple files** (e.g., "generate tests for th 2. Read the source file 3. Write the test file. **The very first line must be the AI attribution comment** (see "AI attribution comment" rule below). Write this comment line *before* any imports or other code. 4. Run `get_errors` on the test file and fix every lint/compile error -5. Call `run_tests` with `include_coverage=true` — tests must pass -6. Meet the coverage target (see Coverage improvement loop) -7. **Only now** move to the next file and repeat from step 1 +5. Call `review_test` on the test file and **resolve every `block` finding** by fixing the test. Address `warn` findings or note why they stand. +6. Call `run_tests` with `include_coverage=true` — tests must pass +7. Meet the coverage target (see Coverage improvement loop) +8. **Only now** move to the next file and repeat from step 1 -If you catch yourself about to call `generate_test` a second time before step 6 has completed for the previous file, **STOP** and finish the previous file first. This applies even when the files are structurally similar — every file gets its own full cycle. +If you catch yourself about to call `generate_test` a second time before step 7 has completed for the previous file, **STOP** and finish the previous file first. This applies even when the files are structurally similar — every file gets its own full cycle. ## MCP tools @@ -48,6 +49,7 @@ If you catch yourself about to call `generate_test` a second time before step 6 | #tool:unittest-mcp `generate_tests_batch` | Scan a **folder** to find source files that need tests. | | #tool:unittest-mcp `inspect_coverage` | Read existing coverage artifacts without re-running tests. Never use as validation for current changes. Pass `source_file` for per-file detail only after fresh coverage exists or when explicitly inspecting existing artifacts. | | #tool:unittest-mcp `find_test_files` | Discover test files in a directory. | +| #tool:unittest-mcp `review_test` | Run the local protective-test gate (static analysis) on a single test file. Call after writing/editing a test; **resolve every `block` finding** before reporting done. | ## Rules @@ -78,8 +80,9 @@ If you catch yourself about to call `generate_test` a second time before step 6 - **Run tests + coverage / validate coverage:** Call `run_tests` with `include_coverage=true`. This is the only tool that validates the current code by executing tests. - **Run tests + inspect coverage gaps:** Call `run_tests` with `include_coverage=true`, then call `inspect_coverage` with the same `root_dir` only if you need uncovered-line/branch detail from that fresh run. - **Run tests with coverage (no explicit inspect request):** Call `run_tests` with `include_coverage=true`. If `coverage.met` is `false`, call `inspect_coverage` automatically. Do not offer coverage work as optional when a coverage target applies. -- **Create tests for a single file:** Call `generate_test` **immediately as your first action**. Do NOT read the source or test file first (exception: Python — brief `file_search` to check test folder layout is OK). Detection: request mentions a specific file with extension (e.g., `user.ts`, `service.py`). +- **Create tests for a single file:** Call `generate_test` **immediately as your first action**. Do NOT read the source or test file first (exception: Python — brief `file_search` to check test folder layout is OK). Detection: request mentions a specific file with extension (e.g., `user.ts`, `service.py`). You **may** pass `test_type` when the intent is obvious (React component/ReactView → `integration`; pure helper → `unit`), but you are **not** required to read source to decide — otherwise omit it and the server resolves `auto`. - **Improve an existing test file:** Call `generate_test` with both `source_file_path` and `test_file_path`. Do NOT read the test file before calling. Apply additive improvements only. +- **Review/validate a test file (protective gate):** Call `review_test` with `test_file_path` (and `source_file_path` when known). Use it after writing or editing any test file, and whenever the user asks to "review", "validate", or "check" a test. Resolve every `block` finding before reporting done. - **Create tests for a folder/multiple files:** See the **"CRITICAL — Process multiple files ONE AT A TIME"** section at the top of this document. For folder requests, call `generate_tests_batch` first to discover the file list, then process each file sequentially through its full cycle before starting the next. - **Inspect existing coverage only:** Call `inspect_coverage` immediately only when the user explicitly asks to read existing coverage artifacts or coverage has already been generated in the current workflow. Do not use it to answer whether current tests pass. @@ -101,8 +104,15 @@ Use the repo's existing test location conventions (`__tests__`, `tests/`, coloca 2. Synthesize concrete test code from the guidance. 3. Create or update the test file. **Line 1 must be the AI attribution comment** — see Rule 4 for the exact text and per-language syntax. This is a required acceptance criterion, not an optional nicety. 4. Run `get_errors` and iteratively fix **all** lint/compile errors until none remain. Do this **before** proceeding. -5. Run tests via `run_tests` with `include_coverage=true` and follow the coverage improvement loop (Section 5) until target coverage is met for the requested scope. -6. Do **not** end with "Want me to add coverage-focused tests?" when coverage is below target — coverage completion is required before reporting done. +5. Call `review_test` on the test file and **resolve every `block` finding** before proceeding — these are the over-mocking, coercion, and weak-assertion patterns the gate exists to stop. Treat `warn` findings as improvements to make or consciously justify. +6. Run tests via `run_tests` with `include_coverage=true` and follow the coverage improvement loop (Section 5) until target coverage is met for the requested scope. +7. Do **not** end with "Want me to add coverage-focused tests?" when coverage is below target — coverage completion is required before reporting done. + +**Protective-test acceptance criteria (satisfy these before reporting done):** +- Render the **real** first-party component/ReactView with its real children, hooks, and context — do **not** mock first-party components, hooks, context, or pure helpers. +- Mock **only** external/platform/network boundaries — network/data/ARM, host-platform services & hooks resolved from a host channel/registry/DI container absent in the test env (symptom: a "No service registered"/missing-provider error), jsdom-unsupported browser APIs, and nondeterminism (time/random/uuid). **First-party app code is never a boundary.** If a platform/UI module cannot run in jsdom, use a faithful double that preserves labels, roles, values, and interactions (never `() => null` or stub `
`s). +- **Reuse shared mocks; hoist recurring ones.** Before adding a boundary mock, check whether the repo already declares it globally (Jest `setupFilesAfterEnv`/`testSetup`, pytest `conftest.py`, .NET base fixtures) and existing test helpers, and reuse those; if the same boundary mock recurs across files, hoist it to that shared setup instead of redeclaring it per file. +- Assert exact values with `toStrictEqual` — one assertion on the whole object, not many single-field asserts (it also catches type / `undefined` / sparse-array mismatches `toEqual` misses); do **not** use `toMatchObject`/`*Containing` as the primary assertion (one unavoidably-varying field, e.g. a timestamped name, may use `expect.stringContaining`); no `as any`/`as unknown as`; assert the value the code returns, not that a function "was called"; keep tests isolated (no mock/variable shared across tests); each test must fail if the behavior under test breaks; the test title must match what it asserts. **Path heuristics:** To find the test file from source, examine the repo's existing test structure (`__tests__`, `tests/`, colocated). To find the source from a test file, remove `.test.`/`.spec.`, move out of `__tests__` (JS/TS), `tests/` (Python), or remove `Tests` suffix (C#). @@ -119,7 +129,7 @@ Use the repo's existing test location conventions (`__tests__`, `tests/`, coloca ### 5. Coverage improvement loop -Do NOT report task completion until coverage for the requested scope meets the configured target (from MCP settings/tool response). +Do NOT report task completion until **all** hold for the requested scope: (a) coverage meets the configured target (from MCP settings/tool response), (b) `review_test` reports **zero `block` findings** on every test file you created or changed, and (c) you have answered `review_test`'s self-review checklist. Coverage percentage alone is not sufficient — a high-coverage test that mocks first-party code or asserts nothing still fails the gate. **Important:** When testing a single file, start with `run_tests` using `include_coverage=true`, `scope='file'`, and an explicit `test_pattern` whenever possible. If the result still needs uncovered-line detail, then call `inspect_coverage` with `source_file` for that same source file. Do not call `inspect_coverage` first when validating current changes. @@ -151,7 +161,18 @@ REPEAT (max 5 iterations): - When `source_file` is provided, LCOV is preferred (has line-level detail) over Istanbul summary. - Interpret the per-file data directly to identify low-coverage files or target specific uncovered paths. -### 7. General rules +### 7. `review_test` — protective-test gate + +`review_test` runs a deterministic static analysis over a single test file and returns structured findings. It does **not** execute tests; run it **in addition to** `run_tests`, after the file is lint-clean. + +- **When:** after writing or editing any test file, and whenever the user asks to review/validate/check a test. +- **Inputs:** `test_file_path` (required), `source_file_path` (optional but improves results), `framework`/`language` (optional). +- **Findings:** each has a `severity` — `block` (must fix), `warn` (should fix), or `info` — plus a `ruleId`, `message`, and often a `suggestedFix`. A finding blocks only when the pattern destroys the test's protection: mocking the module under test (`no-mock-system-under-test`), an `as any`/`as unknown as` cast on the value being asserted or the value a mock returns, and a partial matcher that is a test's only value assertion. The same patterns elsewhere — setup casts, casts used to exercise invalid input, and partial matchers pinning call arguments — are reported as `warn`. Remaining rules cover literal `as T` data coercion, was-called-only assertions, render-without-interaction, and pointless constant snapshots. +- **Self-review checklist:** `review_test` also returns one question per semantic dimension the static rules cannot judge (assertion strength, mocking hygiene, encapsulation, structure, determinism, setup-to-assertion ratio, what NOT to test). You are already the reviewing model — answer each against the test you wrote and fix any failure before reporting done. +- **Acceptance:** **resolve every `block` finding** before reporting done by fixing the test. Do not add protective-test suppression comments to generated tests. +- `review_test` is complementary to `run_tests`: a test can pass and still fail the gate (e.g., it mocks the system under test). Both must be green before you report done. + +### 8. General rules - If 3 consecutive test runs fail, re-read config and retry once. - If source path inference fails, ask the user. diff --git a/CHANGELOG.md b/CHANGELOG.md index d9cbc94..249b1b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,14 +5,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [0.7.0] - 2026-09-09 ### Added +- Select multiple containers across different pods at once without reconnecting +- Select an entire pod (all of its containers) with a single checkbox, including a partial (indeterminate) selection state +- Word-wrap long log lines vertically instead of overflowing horizontally + ### Changed +- Pod/container picker is now multi-select with removable chips +- Changing the selected containers automatically reconnects the stream +- Log viewer measures real row heights so wrapped lines scroll accurately + ### Fixed +- Clearing the container selection now actually clears the stored container value +- Virtualized log rendering accounts for variable-height (wrapped) rows, preventing scroll drift and skipped rows + ## [0.6.0] - 2026-08-10 ### Added diff --git a/frontend/src/components/common/PodContainerSelect.jsx b/frontend/src/components/common/PodContainerSelect.jsx index f5fddd8..0eab0c8 100644 --- a/frontend/src/components/common/PodContainerSelect.jsx +++ b/frontend/src/components/common/PodContainerSelect.jsx @@ -1,10 +1,12 @@ /** - * Pod/container selector. Single-select: - * click a pod to expand its containers, click a container to select it. + * Pod/container selector. Multi-select: + * click a pod to expand its containers, toggle a pod checkbox to select + * ALL containers in that pod, or toggle individual container checkboxes. + * Multiple containers across different pods can be selected at once. * * Props: * options - [{ pod, containers: ['c1', ...] }] - * selected - ['pod/container'] (or [] / ['pod']) + * selected - ['pod', 'pod/container', ...] * onChange - (nextSelected: string[]) => void */ import PropTypes from 'prop-types'; @@ -13,34 +15,48 @@ import { memo, useState, useMemo, useRef } from 'react'; function PodContainerSelectComponent({ options = [], selected = [], onChange, idPrefix }) { const [open, setOpen] = useState(false); const [filter, setFilter] = useState(''); - const [expandedPod, setExpandedPod] = useState(null); + const [expandedPods, setExpandedPods] = useState(() => new Set()); const containerRef = useRef(null); - const selectedValue = selected[0] || ''; - const [selectedPod, selectedContainer] = selectedValue.split('/'); - const selectedPodFull = selectedPod && !selectedContainer ? selectedPod : selectedValue; - const filteredPods = useMemo(() => { const term = filter.toLowerCase(); return options.filter((o) => o.pod.toLowerCase().includes(term)); }, [options, filter]); - const togglePod = (pod) => { - setExpandedPod(expandedPod === pod ? null : pod); - setFilter(''); + const togglePodExpand = (pod) => { + setExpandedPods((prev) => { + const next = new Set(prev); + if (next.has(pod)) next.delete(pod); + else next.add(pod); + return next; + }); }; - const selectContainer = (pod, container) => { - onChange([`${pod}/${container}`]); - setOpen(false); - setExpandedPod(null); + const toggleValue = (value) => { + if (selected.includes(value)) { + onChange(selected.filter((v) => v !== value)); + } else { + onChange([...selected, value]); + } + }; + + // Toggle all containers of a pod on/off + const togglePod = (pod, containers) => { + const allValues = containers.map((c) => `${pod}/${c}`); + const allSelected = allValues.every((v) => selected.includes(v)); + if (allSelected) { + onChange(selected.filter((v) => !allValues.includes(v))); + } else { + const missing = allValues.filter((v) => !selected.includes(v)); + onChange([...selected, ...missing]); + } }; - const clear = (e) => { + const clearAll = (e) => { e.stopPropagation(); onChange([]); setOpen(false); - setExpandedPod(null); + setExpandedPods(new Set()); }; const id = idPrefix ? `${idPrefix}-pod-container` : 'pod-container'; @@ -53,30 +69,35 @@ function PodContainerSelectComponent({ options = [], selected = [], onChange, id
{ setOpen(!open); setFilter(''); }} role="combobox" aria-expanded={open} aria-haspopup="listbox" tabIndex={0} > - {selectedValue ? ( - - {selectedPod} - {selectedContainer && <>/{selectedContainer}} - - + {selected.length > 0 ? ( + selected.map((value) => { + const [pod, container] = value.split('/'); + return ( + + {pod} + {container && <>/{container}} + + + ); + }) ) : ( - Select a pod + Select pods/containers )} - +
@@ -92,11 +113,11 @@ function PodContainerSelectComponent({ options = [], selected = [], onChange, id placeholder="Filter pods..." autoFocus /> - {selectedValue && ( + {selected.length > 0 && ( @@ -106,24 +127,32 @@ function PodContainerSelectComponent({ options = [], selected = [], onChange, id
No pods
)} {filteredPods.map(({ pod, containers }) => { - const isExpanded = expandedPod === pod; - const isSelected = selectedPodFull === pod; + const isExpanded = expandedPods.has(pod); + const podContainerValues = containers.map((c) => `${pod}/${c}`); + const selectedCount = podContainerValues.filter((v) => selected.includes(v)).length; + const allPodSelected = containers.length > 0 && selectedCount === containers.length; + const somePodSelected = selectedCount > 0 && !allPodSelected; return (
togglePod(pod)} + className={`flex items-center gap-2 px-3 py-1.5 cursor-pointer hover:bg-gray-600 ${selectedCount > 0 ? 'bg-green-900/30' : ''}`} + onClick={() => togglePodExpand(pod)} role="option" - aria-selected={isSelected} + aria-selected={selectedCount > 0} > - {pod} - {isSelected ? ( - - - - ) : ( - {containers.length} ctr - )} + { if (el) el.indeterminate = somePodSelected; }} + onChange={() => {}} + onClick={(e) => { e.stopPropagation(); togglePod(pod, containers); }} + className="shrink-0" + aria-label={`Select all containers of ${pod}`} + /> + {pod} + + {allPodSelected ? 'all' : somePodSelected ? `${selectedCount}/${containers.length}` : `${containers.length} ctr`} +
{isExpanded && (
@@ -132,19 +161,21 @@ function PodContainerSelectComponent({ options = [], selected = [], onChange, id )} {containers.map((c) => { const value = `${pod}/${c}`; - const isContainerSelected = selectedValue === value; + const isContainerSelected = selected.includes(value); return (
{ e.preventDefault(); e.stopPropagation(); selectContainer(pod, c); }} + className={`flex items-center gap-2 px-6 py-1 cursor-pointer hover:bg-gray-600 ${isContainerSelected ? 'bg-green-900/30' : ''}`} + onMouseDown={(e) => { e.preventDefault(); e.stopPropagation(); toggleValue(value); }} > + {}} + className="shrink-0" + aria-label={`Select ${value}`} + /> {c} - {isContainerSelected && ( - - - - )}
); })} diff --git a/frontend/src/components/common/__tests__/PodContainerSelect.test.jsx b/frontend/src/components/common/__tests__/PodContainerSelect.test.jsx index 38d6167..1a6c2c9 100644 --- a/frontend/src/components/common/__tests__/PodContainerSelect.test.jsx +++ b/frontend/src/components/common/__tests__/PodContainerSelect.test.jsx @@ -10,7 +10,7 @@ const options = [ describe('PodContainerSelect', () => { it('shows placeholder when nothing selected', () => { render( {}} idPrefix="t" />); - expect(screen.getByText(/Select a pod/i)).toBeInTheDocument(); + expect(screen.getByText(/Select pods\/containers/i)).toBeInTheDocument(); }); it('shows selected pod/container as a chip', () => { @@ -30,6 +30,17 @@ describe('PodContainerSelect', () => { expect(onChange).toHaveBeenCalledWith(['web-1/sidecar']); }); + it('adds to the selection instead of replacing it when picking another container', () => { + const onChange = vi.fn(); + render(); + + fireEvent.click(screen.getByRole('combobox')); + fireEvent.click(screen.getByText('api-1')); + fireEvent.mouseDown(screen.getByText('api')); + + expect(onChange).toHaveBeenCalledWith(['web-1/main', 'api-1/api']); + }); + it('removes selection when the x is clicked', () => { const onChange = vi.fn(); render(); @@ -37,4 +48,24 @@ describe('PodContainerSelect', () => { fireEvent.click(screen.getByLabelText('Remove api-1/api')); expect(onChange).toHaveBeenCalledWith([]); }); + + it('lets the user select all containers of a pod via the pod checkbox', () => { + const onChange = vi.fn(); + render(); + + fireEvent.click(screen.getByRole('combobox')); + fireEvent.click(screen.getByLabelText('Select all containers of web-1')); + + expect(onChange).toHaveBeenCalledWith(['web-1/main', 'web-1/sidecar']); + }); + + it('clears all containers of a pod when deselecting the pod checkbox', () => { + const onChange = vi.fn(); + render(); + + fireEvent.click(screen.getByRole('combobox')); + fireEvent.click(screen.getByLabelText('Select all containers of web-1')); + + expect(onChange).toHaveBeenCalledWith([]); + }); }); diff --git a/frontend/src/components/logs/LogViewer.jsx b/frontend/src/components/logs/LogViewer.jsx index d4a0664..a2a2d69 100644 --- a/frontend/src/components/logs/LogViewer.jsx +++ b/frontend/src/components/logs/LogViewer.jsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useRef, useState, useMemo } from 'react'; import PropTypes from 'prop-types'; const ROW_HEIGHT = 20; @@ -18,6 +18,10 @@ export function LogViewer({ const containerRef = useRef(null); const followedRef = useRef(true); const [viewport, setViewport] = useState({ scrollTop: 0, height: 0 }); + // Measured row heights keyed by log id, so wrapped rows contribute their + // real height to the virtualized geometry. Stored in state so render-phase + // computations (prefix sums) can read them without touching a ref. + const [rowHeights, setRowHeights] = useState(() => new Map()); // Track scroll position and viewport size. Mark whether user is pinned to // the bottom so incoming logs only scroll if they're still at the tail. @@ -45,6 +49,17 @@ export function LogViewer({ }; }, [autoScroll]); + // Reset measurements when the underlying log set changes (e.g. filter + // changes or a fresh stream) so stale heights never apply to new content. + // Adjusting state during render is the React-sanctioned pattern for + // deriving state from a prop change. + const [prevFirstId, setPrevFirstId] = useState(null); + const firstId = logs[0]?.id; + if (firstId !== prevFirstId) { + setPrevFirstId(firstId); + setRowHeights(new Map()); + } + // Jump to the tail whenever new logs arrive while pinned to the bottom. const prevLengthRef = useRef(0); useEffect(() => { @@ -57,6 +72,14 @@ export function LogViewer({ followedRef.current = true; }, [logs]); + // Re-pin to the tail after measurements change the total height while the + // user is still following the stream. + useEffect(() => { + const el = containerRef.current; + if (!el || !followedRef.current) return; + el.scrollTop = el.scrollHeight; + }, [rowHeights]); + const scrollToTop = () => { containerRef.current?.scrollTo({ top: 0, behavior: 'smooth' }); }; @@ -78,13 +101,57 @@ export function LogViewer({ return colors[level?.toLowerCase()] || 'text-gray-300'; }; - // Compute the visible slice from the current scroll position. - const totalHeight = logs.length * ROW_HEIGHT; - const firstVisible = Math.floor(viewport.scrollTop / ROW_HEIGHT); - const visibleCount = Math.ceil(viewport.height / ROW_HEIGHT) + OVERSCAN * 2; - const start = Math.max(0, firstVisible - OVERSCAN); - const end = Math.min(logs.length, start + visibleCount); - const visibleLogs = logs.slice(start, end); + // Record a rendered row's real height so wrapped lines contribute their + // actual size to the virtualized geometry. + const measureRow = (el, id) => { + if (!el) return; + const h = el.offsetHeight; + setRowHeights((prev) => { + if (prev.get(id) === h) return prev; + const next = new Map(prev); + next.set(id, h); + return next; + }); + }; + + // Prefix sums over measured heights (fallback to ROW_HEIGHT for rows not + // yet rendered/measured) so wrapped rows contribute their real height. + const prefix = useMemo(() => { + const arr = new Float64Array(logs.length + 1); + for (let i = 0; i < logs.length; i++) { + arr[i + 1] = arr[i] + (rowHeights.get(logs[i].id) ?? ROW_HEIGHT); + } + return arr; + }, [logs, rowHeights]); + + const totalHeight = prefix[logs.length]; + + // Locate the visible slice: binary search for the first row at/after the + // scroll offset, then walk forward until the viewport + overscan budget. + const visSlice = useMemo(() => { + if (logs.length === 0) { + return { start: 0, end: 0, padTop: 0, padBottom: 0 }; + } + let lo = 0; + let hi = logs.length; + while (lo < hi) { + const mid = (lo + hi + 1) >> 1; + if (prefix[mid] <= viewport.scrollTop) lo = mid; + else hi = mid - 1; + } + const start = Math.max(0, lo - OVERSCAN); + const budget = viewport.height + OVERSCAN * ROW_HEIGHT; + let end = start; + while (end < logs.length && prefix[end + 1] - prefix[start] <= budget) end++; + return { + start, + end, + padTop: prefix[start], + padBottom: totalHeight - prefix[end] + }; + }, [prefix, totalHeight, viewport.scrollTop, viewport.height, logs.length]); + + const visibleLogs = logs.slice(visSlice.start, visSlice.end); return (
@@ -111,7 +178,7 @@ export function LogViewer({ {/* Log Content */}
{logs.length === 0 ? (
@@ -120,43 +187,34 @@ export function LogViewer({
) : ( <> -
-
- {visibleLogs.map((log, idx) => { - const absoluteIdx = start + idx; - return ( -
- - [{log.pod}] - - {log.level && ( - - {log.level.toUpperCase()} - - )} - - {log.message || log.text || ''} - -
- ); - })} -
-
+