Repository navigation
refactor(data-handling): rename the statistics panel to Data Handling and split it into step components and feature services - #56
Open
KFilippopolitis wants to merge 32 commits into
Conversation
…ingComponent Moves src/app/pages/experiment-studio/statistic-analysis-panel to data-handling/, renames StatisticAnalysisPanelComponent to DataHandlingComponent (selector app-data-handling) and updates the experiment studio wiring, specs and docs. No behavior change.
…e component data-handling.models.ts holds the pipeline types and SUMMARY_GUIDE_ANCHORS; utils/ holds the number formatting, the preprocessing documentation builder and the transformation draft factory, each with a focused spec. No behavior change.
…ipComponent details[app-how-to-strip] replaces the #howToStrip ng-template. It attaches to the <details> element itself, so the DOM keeps the same shape and parent rules such as `.workflow-section-body.open > *` still reach it. Its CSS moved with its markup; the station-card margin uses :host-context. No behavior or visual change.
…erminalComponent The "next run will use" card, the hand-off button and the unapplied-changes guard move to data-handling/pipeline-terminal/. The component is attached to the card element (div[app-pipeline-terminal], host class pipeline-terminal-card) so the pipeline column, the studio reveal animation and every selector see the same DOM as before. It is presentational: the parent passes the retention, counts and preprocessing line in and handles continue / apply / discard / stay. Its rules move with it; the shared width rule is split so the parent keeps .pipeline-node.is-compact and the child owns :host(.pipeline-terminal-card). Computed styles and screenshots match master.
…rkspaceComponent The variable browser / statistics / charts / histogram / export workspace that the Raw data snapshot, the Raw preview and the Processed preview all render moves from an ng-template outlet to data-handling/summary-workspace/. It attaches to each section's body element (div.workflow-section-body[app-summary-workspace]), so the DOM, the studio reveal animation, data-guide anchors and spec selectors are unchanged. The workspace's state stays on DataHandlingComponent; sections reach it through the new DATA_HANDLING token (data-handling.context.ts) and use Eager change detection, so they refresh exactly when the inline template did. Cleanups in the moved code: - the duplicated Numerical / Categorical variable groups and the three tab buttons are rendered from typed constants (VARIABLE_GROUPS, SUMMARY_TABS); - coverage is computed once per variable button (@let) instead of twice; - the skeleton grid uses named cell arrays instead of inline number literals; - createEmptySummary becomes the pure utils/summary-view.utils emptySummaryView. CSS: rules for elements that only live in the workspace moved with it; shared rules (tables, empty states, search, button:disabled) are repeated in the panel's cascade order. The node-frame rule becomes :host-context(.pipeline-node). Computed styles and screenshots match master in all harness scenarios. Specs: SummaryWorkspaceComponent (10) and emptySummaryView (2).
Moves the pure part of the statistic summary out of DataHandlingComponent into utils/statistic-summary.utils.ts and types it with the existing VariableStats model instead of any: - pivotByDataset: one table per variable from describe's featurewise rows. The metric list (SUMMARY_METRICS) now uses the payload's own keys, which replaces a six-level ternary that translated num_datapoints/num_missing back to num_dtps/num_na; the counts / categories / full-metrics branches share one row builder. - variableLabel, isCategoricalVariable, enumLabelMap, categoryCounts and pooledTotalRows: small helpers the component re-implemented inline (the public variableLabel / isCategoricalVariable methods delegate, so templates and specs are unchanged). - SummaryView.featurewiseRows is VariableStats[] (was any[]), which removes the remaining `any` casts in buildSummaryFromResponse, buildDistributionState, buildHistogramFromDescribeCounts and getSummaryTotalRows. Rendered tables are unchanged (harness: identical DOM, styles and screenshots). Spec: statistic-summary.utils (9).
…ocessing steps hasPendingMissingChanges and hasPendingOutlierChanges each re-implemented the per-variable comparison that prepVariableHasPendingChange already did (and walked a code set built from three sources that always reduced to the current variables). Each step now has one per-variable check (missingRuleHasPendingChange, outlierRuleHasPendingChange); the step-level getters are `some()` over it, and the "edit, else step default while nothing is applied" resolution both steps used is the pure utils/preprocessing-state.utils pendingOrDefault. No behaviour change: same variables, same comparison, same implicit-default rule. Spec: pendingOrDefault (3).
Decision log entry for how Data Handling is split (attribute-hosted sections, DATA_HANDLING context + Eager change detection, CSS repeated in cascade order, pure utils), and module-index / conventions pointers to the new pieces.
…partials The summary workspace repeated 22 rules of the panel's stylesheets. They now live once in data-handling/styles/ and are @imported by both: - statistics-panel.shared.css: the statistics panel frame and code chip - search-and-empty-states.shared.css: inline search fields and empty states - tables.shared.css: data tables, the statistics table frame, row errors, disabled buttons (and their 768px overrides) The panel imports each partial at the top of the sheet the rules came from, so they move only within that file; selector lists that mixed shared and panel-only selectors (.preprocessing-search/.table-input/.table-select, .empty-state, button:disabled, .data-table td:first-child) were split so only the shared selector moved. The workspace imports all three ahead of its own rules (no rule of its own targets the same element at equal specificity before them). summary-workspace.component.css: 16.4 kB -> 12.5 kB. Harness: identical DOM, computed styles and screenshots against master.
…position The panel located each derived-column category's filter builder through @ViewChildren('ruleModal') and a running index over drafts x rules, which only works while every builder is rendered by the panel's own template, in card order. data-handling/transformation/rule-builder-registry.ts adds the seam that lets the categorical card move into its own component: - RuleBuilderRegistry (provided by the panel): rule -> live builder; - RuleBuilderForDirective (app-filter-config-modal[ruleBuilderFor]): the builder registers itself for the rule it edits, follows a row's rule changes and unregisters on destroy (ngOnChanges, so it is findable right after detectChanges, which addTransformationRule relies on). commitTransformationRuleFilters, finishTransformationRule and addTransformationRule look the builder up by rule (ruleModalAt -> ruleBuilderAt); QueryList/ViewChildren are gone from the panel. Specs: the 19 sites that assigned fake QueryLists now call a local useRuleBuilders(component, [...]) helper (same positional fakes, registered by rule); the two that read real builders use component.ruleBuilders.get(rule). New rule-builder-registry spec (3). Harness: identical against master.
@ViewChild Six @ViewChild(#...Section) refs and the sectionElement switch become a SectionAnchors registry (provided by the panel) fed by a property-bound [dhSectionAnchor] directive, so a section's scroll target can live inside a station component. No DOM change (property binding), same scroll behaviour. Spec: section-anchors (1).
…tionComponent Step 1 (read-only snapshot: header, Records out, the source summary workspace) moves to data-handling/source-data-station/, attached to its pipeline-node section (section[app-source-data-station]); the panel keeps the section element and its state classes. Its scroll target registers through [dhSectionAnchor]. The step chrome every station shares (header, icon, title, badge, records out, body, section frame) moves once into styles/pipeline-node.shared.css, imported by the panel at the top of its first sheet (checked: no equal-specificity rule it used to override sits above it) and by the station. The two rules about the snapshot itself move with it as :host(.pipeline-node.is-readonly ...) rules; .pipeline-node .workflow-section-body is repeated for the station as :host(.pipeline-node) .workflow-section-body. Spec: SourceDataStationComponent (3). Harness: identical against master.
…ationComponent Step 2 (header, folded rule overview, rule builder card with preview/apply, and the Raw preview it feeds) moves to data-handling/cohort-filter-station/, attached to its pipeline-node section (section[app-cohort-filter-station]); the #filterModal template ref and every call that takes it now live in the station. The remaining step chrome it shares with the other steps (Remove Step button, status badge variants, the folded sub-step tree) joins styles/pipeline-node.shared.css (moved to the top of the panel's first sheet; the one equal-specificity overlap the check reports, `.btn-add-step i`, never matches the same element). The match chip rules move with the station; .pipeline-node .workflow-section-body is repeated as a :host rule. Spec: CohortFilterStationComponent (3). Harness: identical against master (the s4 filter scenario is timing-sensitive under 4 workers and was confirmed on its own).
…ionComponent Step 3 (header, folded sub-step rail with its add rows, the missing-values / outlier / longitudinal station cards, the #prepFooter action bar template and the Processed preview) moves to data-handling/preprocessing-station/, attached to its pipeline-node section (section[app-preprocessing-station]). SummaryWorkspace is now imported by the station only. Styles: the sub-step status badge rules join styles/pipeline-node.shared.css and .glass-feedback (with its feedbackSlideIn keyframes, which emulated encapsulation scopes per stylesheet) joins styles/search-and-empty-states.shared.css; both are used by the panel and the station. The step's own rules move with it. .pipeline-node .workflow-section-body is repeated as a :host rule. Left in the panel on purpose: the .preprocessing-documentation-content .preprocessing-doc-* rules. That markup is injected with [innerHTML], so it never carries the _ngcontent attribute and these rules do not match on master either; it is kept as-is and flagged in the PR rather than changed here. Interim: the station's compiled stylesheet is over the 20 kB warning budget (the build reports it under its first @import); the next commits split the station's sub-steps out and bring it back under. Spec: PreprocessingStationComponent (3). Harness: identical against master.
…preprocessing rails The missing-values, outlier and longitudinal steps each carried a ~60-line copy of the same variable rail (heading + count, search, state groups, rows, no-match state). They now share aside[app-prep-variable-rail] (preprocessing-station/prep-variable-rail/), configured with inputs (step, heading, searchLabel, searchId, emptyHint, groupStates). What really differs per step stays in the step's template: its batch menu is projected (.batch-menu) and its per-row markup is given as typed templates, <ng-template prepRailSlot="lead|trail|action" let-variable> (PrepRailSlotDirective with a context guard, so `variable` is a VariableRow), read through contentChildren() + computed(). The rail reads each row's state label once (@let) instead of three times. The DOM is unchanged: the outlier and longitudinal rails gain a state-default class binding that never applies (only missing values have a Default state), and the longitudinal groups still carry no data-state. The grouping itself moves out of the component into utils/preprocessing-state.utils.ts as groupPreprocessingVariables(variables, stateOf): a table of the four groups in display order plus one pass over the variables, replacing the private buildPreprocessingGroups and its three predicate arguments. Styles: the rail's rules move from the station's sheet as :host(.preprocessing-browser) ... (same specificity once compiled), with the search/empty-state partial and the icon nudge. A copy of .preprocessing-browser .empty-state-block that no longer matched anything in the station is dropped. Specs: PrepVariableRailComponent (3), groupPreprocessingVariables (2). Harness: identical against master.
… own components Each sub-step's workspace (variable rail + detail panel) moves out of the station, attached to its existing <section>: - section[app-missing-values-step]: the "Set all" batch menu and coverage/action rail slots, the action choice, the fill value, and the coverage / distribution / category-frequency evidence; - section[app-outlier-step]: the "Batch" menu and on/off rail switch, the method / clip / fold controls, the bounds strip, the distribution aside and the report; - section[app-longitudinal-step]: the type-label rail and the strategy select. The station keeps the step header, folded rail, the station cards with their docs, add/remove buttons, the longitudinal visit controls and the Processed preview. Quality: - the selected variable's rule and state label are read once per render (@let rule / @let state) instead of 6 / 10 / 2-3 times per step; - the redundant `action === 'constant'` re-check inside the constant branch is gone; - the outlier label clamp (12% / 88%), written out twice inline, is one method, OutlierStepComponent.boundLabelShift(). Styles: the rules the steps share (detail panel, sections, headings, controls, evidence, stat strips, rail action line, batch menu) become styles/preprocessing-step.shared.css, imported by each step; .table-input / .table-select join styles/tables.shared.css (the station also uses them). The conflict check reports no real overlap (three `... i` / `... > span` pairs never match the same element). Each step's own rules move with it; the outlier width rules become :host(.outlier-preprocessing-workspace) .preprocessing-detail-panel. The station drops statistics-panel.shared, which it no longer renders, and its compiled stylesheet is back under the 20 kB budget. Specs: MissingValuesStepComponent (3), OutlierStepComponent (3), LongitudinalStepComponent (2). Harness: identical against master.
…tationComponent Step 4 (header, folded derived-column rail, the nominal column cards, the type chooser and the K-means card) moves to data-handling/transformation-station/, attached to its pipeline-node section (section[app-transformation-station]). The #transformationStats template, used only by the K-means card, moves with it. kmeans-cluster-source/ moves under transformation-station/ (paths only). Each nominal card's editor body (name, category rows with their rule builders, fallback) is CategoricalColumnCardComponent, attached to the card's div.transformation-create and given the draft as a signal input. The rule builders keep registering through [ruleBuilderFor] (RuleBuilderRegistry), so no positional lookups are involved. Quality: - the three inline `draft.x = $event; dh.onTransformationInput()` handlers are rename() / setCategory() / setFallback(); - the card reads the draft once (@let column), which also gives template type narrowing on openRuleIndex; - in the station, each card's status and "has new work" are read once (@let) instead of 3 and 5 times. Styles: the pipeline connector rules join styles/pipeline-node.shared.css (also drawn between derived-column cards); the rule table grid that the K-means block and the nominal card share becomes styles/transformation-rules.shared.css; everything else moves with its owner. The panel drops the imports it no longer uses. Specs: TransformationStationComponent (2), CategoricalColumnCardComponent (3). Harness: identical against master.
…-column drafts The draft list and every question asked of it move out of the 4.3k-line panel into a component-scoped service (transformation-station/transformation-drafts.service.ts, provided by DataHandlingComponent): isShown, status, hasNewWork, configs, payload, hasDuplicateCodes, hasAnyWork, hasPendingChange, rulesNeedFilters, firstHidden, and the in-place edits add / remove / reset / addRule / removeRule. The pure parts live in utils/transformation-draft.utils.ts and are now exported and typed: draftHasWork, hasUnfinishedCategory, transformationConfigFor (returning a TransformationColumnConfig instead of Record<string, unknown>), transformationDraftStatus (a TransformationDraftStatus label union), hasDuplicateColumnCodes and firstFilterCondition. The panel keeps its public members (templates and specs are unchanged; transformationDrafts is a getter/setter onto the service) and keeps the side effects: store writes, category counts, rule builders and change detection. Removed from the panel: transformationConfigs, draftHasWork, hasUnfinishedCategory, firstFilterCondition, transformationConfigPayload and five method bodies now one-line delegations. Specs: TransformationDraftsService (5), transformation draft utils (3).
…sing rule maps The pending and applied rules of the three sub-steps (missing values, outliers, longitudinal strategies) and the session's "applied by the user" flag move into a component-scoped service (preprocessing-station/preprocessing-state.service.ts, provided by DataHandlingComponent), together with the reads and comparisons on them: missingRule / outlierRule (lazy step defaults), hasAppliedMissing, hasAppliedOutlier, isUsingDefaultMissing, missingHasPendingChange, outlierHasPendingChange (built on pendingOrDefault). The pure rule helpers become exported, tested functions in utils/preprocessing-state.utils.ts: defaultMissingRule, emptyMissingRule, isDefaultMissingRule, serializeMissingRule, cloneMissingRules. The panel keeps its member names as getter/setter pairs onto the service (pendingPreprocessingRules, appliedPreprocessingRules, pending/appliedOutlierRules, pending/appliedLongitudinalStrategies), so loading, hydration, apply and persistence are untouched; ruleFor, outlierRuleFor, isPreprocessingVariableUsingDefault and the hasApplied* helpers are one-line delegations, and seven private helpers are gone from the panel. Specs that read the private userPreprocessingApplied now read component.preprocessingState.userApplied. Specs: PreprocessingStateService (4), missing-values rule helpers (2).
…faces The three summary surfaces (Source snapshot, Raw preview, Processed preview), the selection each was loaded for, and the describe-response -> SummaryView assembly move into a component-scoped service (data-handling/statistic-summary.service.ts, provided by DataHandlingComponent): get(kind), loadedKey, rawKeyFor(filterOverride), fromDescribeResponse(response, current, variables) and the box-plot / grouped-bar distribution state it builds (now typed as a DistributionState pick). Pure helpers join utils/summary-view.utils.ts: statisticBlockKey, nextStatisticSelection and selectionKey (was the private buildSelectionKey). The panel keeps rawSummary / processedSummary / sourceSummary and the three key fields as getter/setter pairs onto the service, so requests, histograms and exports are untouched; buildDistributionState, buildSelectionKey, statisticBlockKey and nextStatisticSelection leave the panel, which no longer injects ChartBuilderService. Specs that called the private buildSummaryFromResponse / rawSummaryKeyFor or wrote the private key now use component.statisticSummary; the pipeline spec's summaryOf fixture is typed as a SummaryView. Specs: StatisticSummaryService (2), summary view helpers (2).
… CSS partials Updates the decision-log entry on Data Handling sections (station and sub-step components, the three component-scoped feature services, the registries that replaced view queries, the shared CSS partials), the module index and the stylesheet convention for @import partials.
Ports master's ordered transformation steps and PCA column creator onto the split: the transformation station renders one card per store step, and TransformationDraftsService owns the step-keyed drafts, card state, titles, name-clash checks, removal rules and the chooser.
…tation CSS - remove addTransformationDraft, kmeansClusterStatusLabel and selectedSummaryChartTypeLabel (no callers) - delete the .preprocessing-documentation-content .preprocessing-doc-* rules: the markup is injected with [innerHTML], so emulated encapsulation never matches them - fix members that lost their indentation in earlier extractions
…isticsService The panel keeps only the side effects; the service owns the placeholder shapes, the single describe over every derived column, K-means column views and invalidation. The panel passes its pending preprocessing and labels through ports; the transformation views read the service's signals instead of the panel's plain fields.
… util mapBinsToEnumLabels, withDefaultDropFor, the describe-counts histogram, the failed-response message and the response parser leave the panel as pure functions; the panel keeps the request, caching and change detection.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+14
to
+21
| template: ` | ||
| <aside app-prep-variable-rail class="statistics-browser preprocessing-browser" [step]="step" heading="Variables · action" | ||
| searchLabel="Search preprocessing variables" [searchId]="searchId" [groupStates]="groupStates" emptyHint="Clear the search."> | ||
| <div class="batch-menu">batch</div> | ||
| <ng-template prepRailSlot="lead" let-variable><small class="lead">{{ variable.code }}</small></ng-template> | ||
| <ng-template prepRailSlot="action" let-variable><span class="prep-rail-action">{{ variable.code }}-action</span></ng-template> | ||
| </aside> | ||
| `, |
Comment on lines
+14
to
+21
| template: ` | ||
| <aside app-prep-variable-rail class="statistics-browser preprocessing-browser" [step]="step" heading="Variables · action" | ||
| searchLabel="Search preprocessing variables" [searchId]="searchId" [groupStates]="groupStates" emptyHint="Clear the search."> | ||
| <div class="batch-menu">batch</div> | ||
| <ng-template prepRailSlot="lead" let-variable><small class="lead">{{ variable.code }}</small></ng-template> | ||
| <ng-template prepRailSlot="action" let-variable><span class="prep-rail-action">{{ variable.code }}-action</span></ng-template> | ||
| </aside> | ||
| `, |
| @Component({ | ||
| imports: [SectionAnchorDirective], | ||
| providers: [SectionAnchors], | ||
| template: `@if (shown()) {<div id="target" [dhSectionAnchor]="section()"></div>}`, |
| @Component({ | ||
| imports: [SectionAnchorDirective], | ||
| providers: [SectionAnchors], | ||
| template: `@if (shown()) {<div id="target" [dhSectionAnchor]="section()"></div>}`, |
| @Component({ | ||
| imports: [StubBuilderComponent, RuleBuilderForDirective], | ||
| providers: [RuleBuilderRegistry], | ||
| template: `@for (rule of rules(); track $index) {<app-filter-config-modal [ruleBuilderFor]="rule" />}`, |
| @Component({ | ||
| imports: [StubBuilderComponent, RuleBuilderForDirective], | ||
| providers: [RuleBuilderRegistry], | ||
| template: `@for (rule of rules(); track $index) {<app-filter-config-modal [ruleBuilderFor]="rule" />}`, |
coverageFor, blockCoverage, selectionCoverage, missingShareNote, coverageByDataset, distributionFor and categoryCountsFor only ever read a pivot block, so they become pure functions in utils/summary-coverage.utils.ts (with the severity/percent/bar/title helpers and the group label that sit beside them). The panel keeps one-line delegates so the sections, the studio and the specs call the same members as before. Dead code dropped on the way: the panel's metricLabel map (SUMMARY_METRICS already carries the labels), statisticMetricText/Number/MissingValue and isCountMetric. displayNumber and isCountMetricLabel join the existing format and summary utils. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtXkKpquRRkLrfA8GJBfR3
…ummaryHistogramService ensureHistogramForBlock, refreshActiveHistogramPreview and the histogram preprocessing choice become SummaryHistogramService, provided by the panel next to StatisticSummaryService. The panel hands it what it owns through six ports: the selected block, what a block is plotted as (code, split, variable and split label, the last two read when the answer lands), the pending preprocessing, the Cohort Filtering preview filter, group labels and a redraw. Requests, payloads and cache keys are unchanged. Teardown uses takeUntilDestroyed instead of the panel's destroyed subject. Legacy removed on the way: histogramSubscriptionsByVariable never held a real subscription (only Subscription.EMPTY once a request answered), so it is now histogramSettledByVariable, a plain flag record with the same gate. The clearHistogramSubscriptions calls that ran right before a summary was replaced by a fresh one, and in ngOnDestroy, did nothing and are gone; the one that mattered (the Source refetch) resets the flags in place. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtXkKpquRRkLrfA8GJBfR3
…ExportService exportAllDescriptiveToPDF and exportSummaryToCSV only ever read the surface they export plus the studio and the two export services, so they become SummaryExportService with no ports. The panel keeps both methods as one-line delegates (the summary workspace calls them), and isExporting stays as a getter over the service's exporting signal so the shell's overlay reads the same. The off-screen chart containers stay in the panel's template; the service's doc comment names the class contract the PDF reads. The pure parts (summaryKindLabel, datasetDisplayLabel, labelDatasetColumns, summaryCsvRows) move to utils/summary-view.utils with specs. The PDF path had no spec before; the service spec covers the context rows per surface, the chart containers it picks, and the failure and busy states. Dead after the move and removed: the panel's mipVersion and RuntimeEnvService, pivotBlocksWithDatasetLabels, and its PdfExportService/CsvExportService fields. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtXkKpquRRkLrfA8GJBfR3
… the section layout into a util SectionAnchors already knew each section's scroll target, so it now owns the scroll request too: requestScroll() waits, through an effect, for the render that follows an open/close and a newer request replaces one that has not run. The panel drops its request signal, id counter, effect and scrollIntoView helper; its stage Close actions call requestScroll as before. What goToSection does to the page before its side effects (which flags open, which preview shows, which stage it adds) is a pure function of the section, so it becomes utils/section-layout.utils (sectionLayout, STEP_SECTIONS, CLOSED_SECTIONS, isStepShown) with a spec. isStepOpen, the initial state and collapseAllWorkflowSections use it. The sectionOpen/summaryExpanded/addedSteps signals stay panel fields, which the sections, the studio and the specs read. The repeated 'filters' | 'setup' | 'transformation' unions become StageStep. addStep/removeStep stay: removeStep resets preprocessing and transformation state and would need that state's writers moved with it. Docs record what is left in the shell and why. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtXkKpquRRkLrfA8GJBfR3
… innerHTML The SAST gate blocks on the insecure-innerhtml rule even for static strings in a spec. createElement builds the same off-screen containers the PDF export reads. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtXkKpquRRkLrfA8GJBfR3
The entry claimed every extraction was compared against master. The shell slimming commits after the master merge were not run through the local DOM/computed-style/screenshot harness; their coverage is specs and CI e2e. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtXkKpquRRkLrfA8GJBfR3
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
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
Renames
statistic-analysis-panelto Data Handling (DataHandlingComponent,app-data-handling) and splits the ~4.8k-line component / ~1.8k-line template into one component per pipeline step, per-sub-step components, component-scoped feature services (preprocessing state, summary surfaces, summary histogram, summary export, transformation drafts and statistics) and shared CSS partials. Each extracted piece was also cleaned up as it moved: typed instead ofany, signalinput()s, de-duplicated templates and helpers, shorter methods, and a focused spec for each new piece.Behaviour is meant to be identical. Same DOM, visuals, wiring,
data-guideanchors, e2e selectors, request payloads and sessionStorage keys, and no new dependencies. Every commit is green (npm run verifyplus the specs). Every extraction up to the master merge was compared against master with the visual harness (method below); the later shell-slimming commits were not, see Shell slimming.What each piece owns (file by file)
All paths are under
src/app/pages/experiment-studio/data-handling/.data-handling.component.*DATA_HANDLING,SectionAnchors,RuleBuilderRegistryand the feature servicesChartBuilderServiceno longer injecteddata-handling.models.ts,data-handling.context.tsDATA_HANDLINGtoken that sections use to read the pipelineanyreplaced byVariableRow,PivotBlock,SummaryView, …how-to-strip/(details[app-how-to-strip])input()pipeline-terminal/(div[app-pipeline-terminal])input()/output()summary-workspace/(div.workflow-section-body[app-summary-workspace])@let coverage, skeleton arrays; 3 near-copies became one componentsource-data-station/cohort-filter-station/#filterModaland its calls live in the stationpreprocessing-station/#prepFooteraction bar, longitudinal visit controls, Processed preview…/prep-variable-rail/(aside[app-prep-variable-rail])<ng-template prepRailSlot="lead|trail|action" let-variable>slots (directive with a context guard, read viacontentChildren()+computed())…/missing-values-step/,…/outlier-step/,…/longitudinal-step/(section[app-…-step])@let rule/@let state(the rule was read 6 / 10 times per render); a redundantconstantcheck removed; outlier label clamp is now oneboundLabelShift()method…/preprocessing-state.service.tsmissingHasPendingChange,outlierHasPendingChange,isUsingDefaultMissing, …)transformation-station/#transformationStatstemplate@let(were read 3 / 5 times)…/categorical-column-card/(div.transformation-create[app-categorical-column-card])draftas a signalinput(); inlinedraft.x = $event; …replaced byrename()/setCategory()/setFallback();@let columngives type narrowing…/kmeans-cluster-source/…/transformation-drafts.service.tsTransformationColumnConfigandTransformationDraftStatus…/transformation-statistics.service.tsportsstatistic-summary.service.tsrawKeyFor, describe response →SummaryView(including box-plot / grouped-bar state)DistributionStatesummary-histogram.service.tsports. A request that settled (data or error) is not repeated until its surface is rebuilttakeUntilDestroyed;histogramSubscriptionsByVariable(it only ever heldSubscription.EMPTY) is nowhistogramSettledByVariablesummary-export.service.tsexportingsignal behind the page overlay. No portsutils/summary-view.utils; the PDF path now has a specsection-anchors.tsSectionAnchors+[dhSectionAnchor]; replaces six@ViewChildrefs forgoToSectionscrolling. Also runs the scroll request (requestScroll): it waits for the render that follows an open/close, and a newer request replaces one that has not runtransformation/rule-builder-registry.tsRuleBuilderRegistry+app-filter-config-modal[ruleBuilderFor]. Each category's rule builder is looked up by its rule, replacing the positional@ViewChildrenruleModallookupsutils/*SUMMARY_METRICS, view helpers (selectionKey,nextStatisticSelection,statisticBlockKey), preprocessing state (pendingOrDefault,groupPreprocessingVariables, missing-rule helpers), documentation HTML, transformation drafts, histogram preview parsing, outlier bounds, summary coverage / distribution / category reads, the section layoutgoToSectionapplies, export helpersstyles/*.shared.css@importpartials for rules that several components render:pipeline-node(step chrome, sub-step tree, connectors),statistics-panel,search-and-empty-states(+glass-feedbackand its keyframes),tables(+.table-input/.table-select,button:disabled),preprocessing-step,transformation-rulesDocs: the decision-log entry "Data Handling sections are attribute-hosted components over the pipeline's state", plus
module-indexandconventions.File sizes (panel)
statistic-analysis-panel)data-handling).ts.html.component.css.preprocessing.css.results.cssThe largest new piece is
summary-workspace(271-line template, 12.6 kB CSS). Station templates are 33–289 lines, and component TS files are 19–67 lines, exceptkmeans-cluster-source, which already existed.Tests
npm run verify: OK. The only warnings were already on master: thealgorithm-panelCSS budget, andjszipnot being ESM.c58e3f6: Vitest unit tests, Playwright experiment studio, typecheck / dead code / build, SAST, CodeQL, gitleaks and the dependency scans all pass. (The earlier local Playwright run passed 42.)Visual-equivalence method
A local-only Playwright harness drives the pipeline in 8 scenarios against a master worktree and against this branch:
In each state it records:
Chrome runs with deterministic flags:
--font-render-hinting=none,--disable-font-subpixel-positioning,--disable-lcd-text,--disable-gpu,--force-device-scale-factor=1. The comparison was rerun after every extraction and once more on the head before the shell-slimming commits; all 64 artefacts match master. The "s4 filters" scenario sometimes times out under parallel workers; on its own it passes and matches.Before any CSS rule was moved into a partial (an
@importmust come first in a sheet), a script checked that the move did not cross an equal-specificity rule targeting the same element. Every overlap it flagged was checked by hand and was a false positive.Shell slimming (
f8162datof7d842f)The shell was ~3.6k lines after the master merge and is 3179 now. One commit per piece:
f8162daCoverage reads (coverageFor,blockCoverage,selectionCoverage,missingShareNote,coverageByDataset,distributionFor,categoryCountsFor) are pure functions inutils/summary-coverage.utils.ts. The panel keeps one-line delegates, so the sections and specs call the same members.018b492SummaryHistogramServiceowns the histogram request/cache loop. Payloads, cache keys and request order are unchanged.61728efSummaryExportServiceowns the PDF/CSV export.isExportingstays as a getter over its signal; the off-screen chart containers stay in the panel's template.cd794d5Scroll requests moved intoSectionAnchors; the open/preview/added-stage transition ofgoToSectionisutils/section-layout.utils.ts. ThesectionOpen/summaryExpanded/addedStepssignals stay panel fields.c58e3f6,f7d842fAn export-spec fix for the SAST gate (innerHTMLin a spec, nowcreateElement) and a decision-log correction.Dead code removed on the way: the panel's
metricLabel,statisticMetric*,isCountMetric,mipVersion,pivotBlocksWithDatasetLabels, and theclearHistogramSubscriptionscalls that ran right before a summary was replaced (or inngOnDestroy) and did nothing.Not touched: any
.htmlor.cssfile, request payloads, sessionStorage keys, routes, guards, auth, sharing, deletion, Docker/nginx/CI, runtime env, dependencies.Risk
effectnow lives in a component-scoped service (SectionAnchors). The panel'sscrollIntoViewspecs and a newSectionAnchorsspec pass, but it is the one change with a timing component. Worth a manual click-through of the stepper, "Preview data" and a PDF export.SummaryView.histogramSubscriptionsByVariablewas renamed tohistogramSettledByVariable. It is an internal shape (the models,emptySummaryView,StatisticSummaryService), read by nothing outside the panel.Rollback: each commit builds and passes on its own. Revert in reverse order;
c58e3f6belongs with61728ef.Open questions / risk areas
TransformationDraftsService): drafts are still edited in place. The card mutates the draft object the service holds, and the panel reacts throughonTransformationInput(). Moving this to immutable updates is a follow-up, not part of this behaviour-preserving PR.ruleFor/outlierRuleForwrite while reading: they create the default pending rule on first read, as on master. The new@letreads keep the same order of first reads..preprocessing-documentation-content .preprocessing-doc-*rules never match, because that markup is injected with[innerHTML]and has no_ngcontentattribute. They are kept as they are, in the panel. Fixing them (::ng-deepor a styled wrapper) would be a visible change and needs a decision.*DescribeSeqcounters, the rule maps andemitProgressState, and about a dozen panel methods (43 assignment sites) still write the pending/applied rule maps directly, so the writers have to move intoPreprocessingStateServicetogether.ChangeDetectionStrategy.Eagerwhere they read the panel's plain fields throughDATA_HANDLING. This matches how the inline template refreshed. They can move to OnPush once the panel state becomes signals.🤖 Generated with Claude Code
https://claude.ai/code/session_01EtXkKpquRRkLrfA8GJBfR3