feat(studio): anonymizer builder live preview panel [ASTD-332] - #1005
Merged
Conversation
Contributor
|
marcusds
force-pushed
the
astd-332-anonymizer-live-preview-panel/mschwab
branch
from
August 4, 2026 22:02
4f9806f to
ceed457
Compare
marcusds
marked this pull request as ready for review
August 4, 2026 22:23
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesThe PR adds streamed anonymizer previews, anonymizer record parsing, highlighted record rendering, replacement-map pagination, preview validation, cancellation, logs, and builder controls. Anonymizer preview flow
Sequence Diagram(s)sequenceDiagram
participant AnonymizerBuilderForm
participant useAnonymizerPreview
participant streamAnonymizerPreview
participant PreviewPanel
participant AnonymizerRecordView
AnonymizerBuilderForm->>useAnonymizerPreview: runPreview()
useAnonymizerPreview->>streamAnonymizerPreview: submit PreviewRequest
streamAnonymizerPreview-->>useAnonymizerPreview: streamed preview frames
useAnonymizerPreview-->>PreviewPanel: preview records, logs, and status
PreviewPanel->>AnonymizerRecordView: build and render selected record
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx (1)
96-111: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winResolve the promise if
handleSubmitrejects.
form.handleSubmitreturns a promise. If the resolver throws, neither callback runs and this promise never settles.runPreviewthen awaits forever with no user feedback and no error state.♻️ Proposed fix
new Promise<PreviewRequest | undefined>((resolve) => { - void form.handleSubmit( + form.handleSubmit( (values) => { setSubmitError(undefined); resolve(buildAnonymizerPreviewRequest(values, defaultEntityLabels?.data ?? [])); }, (errors) => { showValidationErrors(errors); resolve(undefined); } - )(); + )().catch(() => { + setSubmitError('Could not validate the form.'); + resolve(undefined); + }); }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx` around lines 96 - 111, Update getPreviewRequest to handle rejection from the promise returned by form.handleSubmit, ensuring the surrounding Promise always settles when the resolver or submission flow throws. Reject or resolve with undefined consistently with the existing error path, and surface the failure through the existing validation/error handling used by runPreview.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/components/AnonymizerRecordView/parse.ts`:
- Around line 136-153: Update the matching logic in the parse flow around
resolveSynthetic and indexOf so repeated replacement text is aligned with its
source entity rather than selecting the first identical occurrence. Use
surrounding source context or edit alignment to map “Teddy met Bobby” to the
replacement occurrence corresponding to Bobby, while preserving unchanged-text
handling; add a regression test covering this scenario.
- Around line 36-43: Update toEntity to validate that start and end are
integers, not merely numbers, before returning an AnonymizerEntity; reject
fractional offsets so slicing and cursor tracking remain consistent. Add a test
covering fractional start or end positions and verify the entity is rejected.
In
`@web/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerPreview.ts`:
- Around line 66-79: The runPreview flow should reserve and store its
AbortController before awaiting getRequest, and move request construction inside
the existing try/error path so rejected builds update error state. In
runPreview, ignore streamed frames and guard the setLogs updater unless that
controller remains abortRef.current, preventing stale runs from overwriting
newer previews. Add a test covering deferred request construction resolving out
of order.
---
Nitpick comments:
In
`@web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx`:
- Around line 96-111: Update getPreviewRequest to handle rejection from the
promise returned by form.handleSubmit, ensuring the surrounding Promise always
settles when the resolver or submission flow throws. Reject or resolve with
undefined consistently with the existing error path, and surface the failure
through the existing validation/error handling used by runPreview.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8c166a57-0e9c-4807-a2f1-1f728cdd4ce3
📒 Files selected for processing (24)
web/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordSkeleton.tsxweb/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordView.test.tsxweb/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordView.tsxweb/packages/studio/src/components/AnonymizerRecordView/HighlightedText.tsxweb/packages/studio/src/components/AnonymizerRecordView/RecordSection.tsxweb/packages/studio/src/components/AnonymizerRecordView/ReplacementMapTable.tsxweb/packages/studio/src/components/AnonymizerRecordView/parse.test.tsweb/packages/studio/src/components/AnonymizerRecordView/parse.tsweb/packages/studio/src/components/AnonymizerRecordView/types.tsweb/packages/studio/src/components/NewDataDesignerJobForm/previewApi.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/PreviewPanel.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/RecordPager.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/constants.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/previewApi.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/previewApi.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerPreview.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/utils.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/utils.tsweb/packages/studio/src/routes/AnonymizerJobDetailRoute/util.tsweb/packages/studio/src/util/guards.tsweb/packages/studio/src/util/lineStream.test.tsweb/packages/studio/src/util/lineStream.ts
Builds the right-hand Preview panel of the anonymizer builder, replacing the "Your records preview will appear here" placeholder. A "Preview" button beside the Source / Model Settings tabs streams the current form config through the anonymizer `/preview` endpoint and renders the result: Original vs Replaced text with inline PII tags, a Replacement Map table, a record pager, loading skeletons, and a collapsible Logs panel. "Full Run" moves into the panel header and still submits the create-job form. The endpoint returns `application/x-ndjson`, not SSE, so this follows the data designer's `streamPreview` shape rather than the SSE reader in `util/sseStream` (which is GET-only). Frames are shape-guarded and unknown `kind` values are dropped rather than surfaced. Replaced-side entity spans are located by searching the replaced text for each synthetic value rather than replaying the original offsets, which drift as soon as a replacement changes length. This mirrors the upstream library's own display path, including its case-insensitive lookup fallbacks and the build-from-replacement-map path used when detection returns no spans. The record view lives under components/AnonymizerRecordView so the job detail page (ASTD-330) and evaluation results (ASTD-322) can reuse it. Signed-off-by: mschwab <mschwab@nvidia.com>
…er [ASTD-332] `useStudioDataViewState` reads `page` from a shared, unprefixed URL param, so the replacement map's page index survives moving the record pager. Paging to a record with a shorter map then sliced past the end and rendered an empty table — with the pager hidden below page size, nothing was left to recover with. Clamp the requested page to the row count. Also drops the left panel footer. The Figma marks that frame hidden and moves its only visible button, Full Run, into the preview header, which left Cancel stranded there on its own. Exit is via the breadcrumb. Signed-off-by: mschwab <mschwab@nvidia.com>
Clicking Preview on an incomplete form jumped to Model Settings even when the
failing field was on Source, hiding the error that explained the refusal.
Two causes. `getPreviewRequest` read `form.formState.errors` after `trigger()`,
but RHF's `formState` is a proxy that only maintains the fields the render body
subscribes to, and this component reads none of it — the lookup returned `{}`.
`[].every()` is then vacuously true, so an empty error set satisfied the
"models are the only failure" test and routed to the wrong tab.
Take the errors from `handleSubmit`'s invalid callback instead, which is what
the Full Run path already does, and require a non-empty field list before
treating models as the sole failure. The tab choice moves into
`tabForValidationErrors` so the empty case is pinned by a test.
Signed-off-by: mschwab <mschwab@nvidia.com>
The Preview button was disabled for the duration of a run, so a preview that turned out to be misconfigured had to be waited out or escaped by navigating away. Swap it for Stop, which aborts the request. The abort is honest rather than cosmetic: closing the connection makes Starlette close the response generator, which cancels the drain task and unwinds the function's task group until the worker's next frame send raises `PreviewMessageDeliveryError`. It is not instant — the anonymizer runs under `abandon_on_cancel=True`, so the worker thread is abandoned rather than joined and an in-flight model call still finishes. A stopped run reports itself as stopped instead of claiming the run returned no records. Partial records already streamed are kept. This departs from the Figma, which greys the button out for the duration. Signed-off-by: mschwab <mschwab@nvidia.com>
…D-332] The entity chips used `Tag`, which is the interactive pill — button-based, large radius, generous padding — so inline text read as chunky and did not match the mock. Code Connect on the Figma `PII Tag` component maps both chips to `Badge` (`kind="solid"` for the value, `kind="outline"` for the label), which is the non-interactive `<span>` label at Label/Bold/sm. The design's wrapper carries no gap, so the pair now sits flush. Same swap for the Replacement Map's Label column, which Code Connect also maps to an outline `Badge`. `entityTagColor` is unchanged — Tag and Badge expose the same seven colours, so the existing category mapping carries over. `EntitiesSection` keeps `Tag`, which is correct there: those chips are removable and interactive. Signed-off-by: mschwab <mschwab@nvidia.com>
…ASTD-332] Log frames arrive one per stream read, too far apart for React to batch, so each one synchronously re-rendered the builder form and the preview panel. That put the new Stop button behind a queue of log renders exactly when a chatty run made it most useful. They now update in a transition. `AnonymizerRecordView` and `HighlightedText` are memoized. Their props are already stable references — the record is built inside a memo — so this actually prevents work rather than adding a comparison that never hits. Also hoists the static skeleton rows out of the render path and folds the output heading into the record memo, since both derive from the same inputs. Trims the branch's comments back to the ones carrying a non-obvious why. Signed-off-by: mschwab <mschwab@nvidia.com>
`PreviewPanel` had grown a skeleton, a record pager and the panel chrome in one file. Splits out `RecordPager`, and moves the skeleton next to the view it mirrors as `AnonymizerRecordSkeleton` — the two render the same three sections and had drifted apart in the same file already. `RecordSection` replaces six hand-written copies of the heading-plus-content block across the view and the skeleton, so they can no longer diverge. The `asObject` guard was written twice on this branch, in `parse.ts` and `previewApi.ts`. Promotes it to `@studio/util/guards` as `asRecord`, matching the name the two pre-existing hand-rolled copies elsewhere in Studio already use; those call sites are left alone as out of scope here. Signed-off-by: mschwab <mschwab@nvidia.com>
…STD-332] The output heading is derived from whichever output column a record carries, which does not exist until results arrive — so a rewrite preview showed "Replaced" over the skeleton for the whole run, then flipped to "Rewritten". The form knows the strategy before the run starts, so it now passes the heading down for the loading state. A loaded record still derives its own, which stays correct if the strategy is changed after a run. Signed-off-by: mschwab <mschwab@nvidia.com>
…es [ASTD-332] An audit of this branch against what Studio already ships turned up four things it had reimplemented. `isAbortError` already exists in `common/AssistantChat/completionUtils`, and its version is the better one — it matches any `Error` whose name is `AbortError` or `APIUserAbortError`, where the copy here only recognised a `DOMException`. Drops the copy and its tests. The NDJSON read loop was duplicated between this preview and the data designer preview, down to the trailing-partial handling. Extracts `readLineDelimitedStream` and points both at it, with a test covering the chunk-boundary cases neither call site exercised. `OUTPUT_SUFFIXES` was declared here and again in `AnonymizerJobDetailRoute`. The record view is the reusable home the ticket asked for, so the job detail route now imports it, and `PreviewPanel` drops its third copy of the literal. `StackedSkeleton` already renders N stacked skeleton lines, so the local `SkeletonBlock` uses it rather than mapping its own. Signed-off-by: mschwab <mschwab@nvidia.com>
ASTD-350 landed, so the anonymizer spec now describes the NDJSON frames and the SDK generates them. Drops the hand-written `PreviewFrame` union and `PreviewLogLevel` in favour of the generated `LogFrame`, `PreviewDatasetFrame`, `TraceDatasetFrame`, `FailedRecordsFrame`, `Heartbeat`, `Done` and `Error`. The runtime guards stay. Orval emits `zod.unknown()` for the response because the media type isn't JSON, and these frames are untrusted network input either way — what the SDK removes is the duplicated type declarations, not the need to validate them. `trace_dataset`'s column keeps its wire name, `original_text_column`, rather than the camelCase alias this file used to apply at the parse boundary. Signed-off-by: mschwab <mschwab@nvidia.com>
…STD-332] buildReplacedEntities located each span with a forward indexOf on the synthetic value, which is not bound to the source entity. When the unchanged text already contained that value, the preview tagged the wrong occurrence — "Bobby" -> "Teddy" in "Teddy met Bobby" mapped to offset 0. Text between entities is untouched, so the gap since the last match predicts the next position; fall back to the forward search only when that alignment misses. useAnonymizerPreview built its request before taking controller ownership, so two runs could resolve out of order and let an older run overwrite a newer preview, and a rejected build never reached the error state. Reserve the controller first, construct inside the try, and drop frames from superseded runs. getPreviewRequest also left its promise unsettled if handleSubmit rejected, hanging runPreview with no feedback. Signed-off-by: mschwab <mschwab@nvidia.com>
main dropped the react-router-dom re-export. Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
force-pushed
the
astd-332-anonymizer-live-preview-panel/mschwab
branch
from
August 5, 2026 05:50
ceed457 to
fe83b2e
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerPreview.test.ts (1)
4-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlace external imports before internal aliases.
Move
@testing-library/reactbefore the@studioimports.
As per coding guidelines, “Group imports: external libraries, internal modules, relative imports in TypeScript.”Proposed fix
import type { PreviewRequest } from '`@nemo/sdk/generated/anonymizer/schema`'; +import { act, renderHook, waitFor } from '`@testing-library/react`'; + import { streamAnonymizerPreview } from '`@studio/routes/AnonymizerBuilderRoute/previewApi`'; import { useAnonymizerPreview } from '`@studio/routes/AnonymizerBuilderRoute/useAnonymizerPreview`'; -import { act, renderHook, waitFor } from '`@testing-library/react`';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerPreview.test.ts` around lines 4 - 7, Reorder imports in the useAnonymizerPreview test so the external `@testing-library/react` import appears before the internal `@nemo` and `@studio` alias imports, preserving the existing imported symbols.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@web/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerPreview.test.ts`:
- Around line 4-7: Reorder imports in the useAnonymizerPreview test so the
external `@testing-library/react` import appears before the internal `@nemo` and
`@studio` alias imports, preserving the existing imported symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0486fd25-8d69-4ecf-bc4e-cbbe12b02211
📒 Files selected for processing (25)
web/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordSkeleton.tsxweb/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordView.test.tsxweb/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordView.tsxweb/packages/studio/src/components/AnonymizerRecordView/HighlightedText.tsxweb/packages/studio/src/components/AnonymizerRecordView/RecordSection.tsxweb/packages/studio/src/components/AnonymizerRecordView/ReplacementMapTable.tsxweb/packages/studio/src/components/AnonymizerRecordView/parse.test.tsweb/packages/studio/src/components/AnonymizerRecordView/parse.tsweb/packages/studio/src/components/AnonymizerRecordView/types.tsweb/packages/studio/src/components/NewDataDesignerJobForm/previewApi.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/PreviewPanel.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/RecordPager.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/constants.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/previewApi.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/previewApi.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerPreview.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerPreview.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/utils.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/utils.tsweb/packages/studio/src/routes/AnonymizerJobDetailRoute/util.tsweb/packages/studio/src/util/guards.tsweb/packages/studio/src/util/lineStream.test.tsweb/packages/studio/src/util/lineStream.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- web/packages/studio/src/components/AnonymizerRecordView/types.ts
- web/packages/studio/src/util/lineStream.ts
- web/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordView.tsx
- web/packages/studio/src/util/guards.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/previewApi.test.ts
- web/packages/studio/src/components/AnonymizerRecordView/HighlightedText.tsx
- web/packages/studio/src/util/lineStream.test.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.ts
- web/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordSkeleton.tsx
- web/packages/studio/src/routes/AnonymizerBuilderRoute/components/RecordPager.tsx
- web/packages/studio/src/routes/AnonymizerBuilderRoute/utils.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/constants.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/utils.test.ts
- web/packages/studio/src/components/NewDataDesignerJobForm/previewApi.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/components/PreviewPanel.tsx
- web/packages/studio/src/components/AnonymizerRecordView/parse.test.ts
- web/packages/studio/src/routes/AnonymizerJobDetailRoute/util.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx
- web/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerPreview.ts
- web/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordView.test.tsx
- web/packages/studio/src/routes/AnonymizerBuilderRoute/previewApi.ts
- web/packages/studio/src/components/AnonymizerRecordView/ReplacementMapTable.tsx
Detection reports code-point offsets the way Python counts them, but JS string indices are UTF-16 code units, so any non-BMP character ahead of a span shifted the highlight — "😀 Alice" at 2..7 rendered " Alic". Remap detected spans once per record; texts without a surrogate pair short-circuit to the input array. The preview endpoint rejects num_records above preview_num_records.max with a 422 instead of clamping, and the bound is imperative so it never reached the OpenAPI spec or the generated schema. Mirror the default client-side so the form catches it before the round trip. ReplacementMapTable clamped only the row slice, leaving the shared page index untouched, so moving from page 3 to a smaller map rendered the last rows under a "page 3 of 2" control. Signed-off-by: mschwab <mschwab@nvidia.com>
Signed-off-by: mschwab <mschwab@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/components/AnonymizerRecordView/parse.ts`:
- Around line 82-86: Validate entity start and end offsets against the original
text/code-point mapping before converting them in unitAt, dropping any entity
with an out-of-range offset instead of mapping it to text.length. Update the
fallback at the detected-entity handling near line 248 to use the converted,
filtered entity list rather than detected.length. Add a regression test covering
😀 with an out-of-range end offset.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fa62aca7-217c-41a9-84dd-158fcdbfe838
📒 Files selected for processing (8)
web/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordView.test.tsxweb/packages/studio/src/components/AnonymizerRecordView/ReplacementMapTable.tsxweb/packages/studio/src/components/AnonymizerRecordView/parse.test.tsweb/packages/studio/src/components/AnonymizerRecordView/parse.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/GenerationSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/constants.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- web/packages/studio/src/routes/AnonymizerBuilderRoute/constants.ts
- web/packages/studio/src/components/AnonymizerRecordView/ReplacementMapTable.tsx
- web/packages/studio/src/components/AnonymizerRecordView/AnonymizerRecordView.test.tsx
- web/packages/studio/src/components/AnonymizerRecordView/parse.test.ts
toUtf16Offsets mapped an offset past the last code point onto text.length, which is exactly the value toSegments' bounds check lets through — so a span the old code rejected became a highlight running to the end of the text. Drop those entities instead, and decide the replacement-map fallback on the converted list, since conversion can now empty it. Signed-off-by: mschwab <mschwab@nvidia.com>
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- import/order configuration ---'
rg -n -A20 -B8 "'import/order'|\"import/order\"" web/eslint.config.js
printf '%s\n' '--- test imports ---'
sed -n '1,70p' web/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerPreview.test.tsLength of output: 3928
You are interacting with an AI system. |
marcusds
added a commit
that referenced
this pull request
Aug 5, 2026
The guideline said "group imports: external libraries, internal modules, relative imports", which is not what `import/order` does. It collapses builtin and external into one group and alphabetizes within it, and the `@nemo` / `@nvidia` / `@studio` aliases resolve as external — so they sort alongside `react` instead of forming an internal group. Reviewers read the guideline, see an alias ahead of a package, and file it as a defect. CodeRabbit did exactly that on #1005, citing "Coding guidelines" as its source. Its suggested reordering fails `--max-warnings 0`. State the rule's real behaviour, and tell CodeRabbit that import order is lint-owned and not reviewable. Signed-off-by: mschwab <mschwab@nvidia.com>
Every streamed log frame appends to logs and re-renders the panel, so the items array and the join over the whole buffer were rebuilt per frame — and slotContent is a prop rather than a render callback, so both ran even with the accordion collapsed. Cost grew with the buffer, making a long run quadratic. Signed-off-by: mschwab <mschwab@nvidia.com>
…332] Bind the replacement map's data columns with col.accessor so they carry a data model rather than being unlinked display columns; the arrow column stays display. enableSorting is false by default in useCustomReactTable, so this adds no sort UI. Drop the explicit displayName assignments. There is no react/display-name rule in the web config and most memo components in studio already omit them. Share the trace row between parse.test.ts and AnonymizerRecordView.test.tsx via testFixtures.ts. The parsed entity list and the wire-shaped final_entities stay separate literals so the parseEntities assertion is not checked against a second implementation of the same mapping. Add a buildReplacedEntities regression case where a duplicate of the synthetic value sits between searchFrom and the true position after a length-changing replacement. The gap prediction in locate() is what keeps this correct; without it the preview would tag the earlier literal. Signed-off-by: mschwab <mschwab@nvidia.com>
steramae-nvidia
approved these changes
Aug 5, 2026
marcusds
enabled auto-merge
August 5, 2026 21:26
ryana
pushed a commit
to ryana/nemo-platform
that referenced
this pull request
Aug 12, 2026
…VIDIA-NeMo#1094) * docs(studio): describe the import order the linter actually enforces The guideline said "group imports: external libraries, internal modules, relative imports", which is not what `import/order` does. It collapses builtin and external into one group and alphabetizes within it, and the `@nemo` / `@nvidia` / `@studio` aliases resolve as external — so they sort alongside `react` instead of forming an internal group. Reviewers read the guideline, see an alias ahead of a package, and file it as a defect. CodeRabbit did exactly that on NVIDIA-NeMo#1005, citing "Coding guidelines" as its source. Its suggested reordering fails `--max-warnings 0`. State the rule's real behaviour, and tell CodeRabbit that import order is lint-owned and not reviewable. Signed-off-by: mschwab <mschwab@nvidia.com> * docs(studio): point at pnpm lint:fix, not eslint directly The repo script carries --report-unused-disable-directives and --max-warnings 0 and uses the pinned ESLint; AGENTS.md already says to go through package scripts rather than invoking tools directly. Signed-off-by: mschwab <mschwab@nvidia.com> --------- Signed-off-by: mschwab <mschwab@nvidia.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
2 participants
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.
Builds the right-hand Preview panel of the anonymizer builder (
/anonymizer/new), replacing the "Your records preview will appear here" placeholder.Closes ASTD-332.
What it does
Preview (beside the
Source | Model Settingstabs) streams the current form config through the anonymizer/previewendpoint and renders:‹ Record 1 of N ›), loading skeletons, empty state, error bannerlogframesWhile a run is in flight the button becomes Stop.
Full Runmoves into the panel header per the Figma and still submits the create-job form — the panel is inside the same<form>. The left panel footer is dropped: Figma marks that frame hidden and moves its only visible button into the preview header, which leftCancelstranded. Exit is via the breadcrumb.Frame types now come from the SDK
Originally this branch hand-wrote the
PreviewFrameunion and its guards, because the spec described the streaming 200 as an empty schema under the wrong media type. #1024 (ASTD-350) fixed that at the source, and this now imports the generatedLogFrame,PreviewDatasetFrame,TraceDatasetFrame,FailedRecordsFrame,Heartbeat,DoneandError.The runtime guards stay deliberately. Orval emits
zod.unknown()for the response because the media type isn't JSON, and frames are untrusted network input regardless — the SDK removed the duplicated declarations, not the need to validate. The streaming reader stays hand-written too; orval doesn't generate incremental readers.Notes for review
Replaced-side spans are searched, not replayed. Each entity is located in the replaced text by scanning forward for its synthetic value. Replaying the original offsets drifts the moment a replacement changes length — a bug the upstream library already hit and worked around, so
parse.tsmirrors itsdisplay.pylogic: the exact → value-only → case-insensitive lookup ladder, plus the build-from-replacement-map fallbacks for when detection returns no spans but the map is populated.PII tags use
Badge, notTag. Code Connect on the FigmaPII Tagcomponent maps toBadge(solidfor the value,outlinefor the label);Tagis the interactive pill and rendered noticeably heavier than the mock.Stop is real, not cosmetic — but not instant. Aborting makes Starlette close the response generator, cancelling the drain task and unwinding the function's task group until the worker's next frame send raises
PreviewMessageDeliveryError. Because the anonymizer runs underabandon_on_cancel=True, the worker thread is abandoned rather than joined, so an in-flight model call still finishes.Validation routing was reading empty errors. Clicking Preview on an incomplete form jumped to Model Settings even when the failing field was on Source.
formStateis a proxy that only maintains what the render body subscribes to, and this component reads none of it — soerrorscame back{}, and[].every()is vacuously true. Errors now come fromhandleSubmit's invalid callback, which is what the Full Run path already used.The shared-URL-param hazard.
useStudioDataViewStatesyncs to unprefixed params (page,page_size,sort, …). That bit inside this PR: with a map that overflows a page, going to?page=2then advancing the record pager to a shorter map sliced past the end and rendered an empty table — with the map's pager hidden below page size, nothing was left to recover with. Clamped, with a regression test confirmed to fail without the fix. The broader collision remains for when ASTD-330 reuses this table alongsideResultsPreviewTable; that wants aparamPrefixoption on the shared hook.Reuse audit. Four things this branch had reimplemented now use what Studio already ships:
isAbortError(common's version also handlesAPIUserAbortError, mine only caughtDOMException), the NDJSON read loop (extracted asreadLineDelimitedStream, shared with the data designer preview and given the chunk-boundary test neither call site had),OUTPUT_SUFFIXES(was declared here and inAnonymizerJobDetailRoute), andStackedSkeleton.The record view lives in
components/AnonymizerRecordView/so the job detail page (ASTD-330) and evaluation results (ASTD-322) can reuse it.Screenshots
Testing
parse,previewApi,lineStream, the record view and the tab/heading helpersRebased onto
mainafter #1024 merged.Summary by CodeRabbit