refactor: remove dead code and share test setup - #1066
Open
lukecotter wants to merge 5 commits into
Open
lukecotter wants to merge 5 commits into
lukecotter wants to merge 5 commits into
Conversation
The mesh renderers replaced the sprite ones: FlameChart uses MeshRectangleRenderer, MeshMarkerRenderer and MeshAxisRenderer, and SearchOrchestrator uses MeshSearchStyleRenderer. No production file imported the sprite path. Deletes AxisRenderer, SearchStyleRenderer, TimelineMarkerRenderer, SpritePool and EventBatchRenderer, plus markers.test.ts and batching.test.ts, which constructed only those classes. TemporalSegmentTree.test.ts and RectangleCache.bucket.test.ts already cover the culling, size classification and bucketing that batching.test.ts exercised. sortMarkersByTimeAndSeverity had no coverage outside markers.test.ts, so its case moves to MarkerProcessor.test.ts. Marker viewport culling, end-time resolution and per-type colour lose their tests. They asserted sprite tint, alpha, x and width — internals of the deleted class. MeshMarkerRenderer keeps visibleIndicators private and had no suite of its own before this change either.
It holds the multi-root workspace prompt. RetrieveLogFile and LogView take workspaceFolders[0], so a multi-root workspace never gets a choice; this is the logic that gives it one.
TimeGridCalculator has no importer. Its two name hits are its own header and a comment banner in timeAxisConstants.ts. MeshAxisRenderer calls selectInterval directly. Its four comments naming the deleted AxisRenderer go with it.
Each symbol was verified as a single rg hit, its own declaration. Dead exports: APEX_METRIC_COLORS and getMetricColor (already deprecated), METRIC_STRIP_TOGGLE_COLORS, MinimapKeyboardCallbacks, validateMarker, MINIMAP_HEIGHT, LABEL_OFFSET_X/Y, EventDetail, createMockEventTree, and the SearchMatch, HeatStripMetricSnapshot, HeatStripTimeSeriesMetric, SegmentTreeQueryResult and Swimlane* types. MetricStripTimeSeries and MetricStripColors' soql/dml/cpu/heap lose their last readers with the symbols above, so they go too. MinimapViewport.setHeatStripReservation only ever set heatStripReservation to its initial 0, so getChartBottom subtracted a constant zero. The shim, the field and the subtraction go together, which leaves getChartBottom a copy of getHeight; its eight callers now call getHeight directly. TimelineFlameChart's isInitialized @State was written, never read. AppConfig declared a Flow timeline colour that lana/package.json does not contribute and sets additionalProperties false against, so it could never be populated. Flow events map to Workflow through LEGACY_CATEGORY_MAP. datagrid-range-filter declared its own FilterRange, byte-identical to the one in tabulator/filters/MinMax.ts that every other consumer imports. APEX_GOVERNOR_LIMITS_DOC had no consumer; the URL moves into the file header so the provenance survives. Dead selectors: .loading-message, .vs-checkbox-label, .header-bar. Every --lana-* token is live, so none go.
Nothing imports it. rollup.config.mjs takes commonjs, json and node-resolve only; scripts/measure/rolldown.config.ts uses rolldown's own resolve.alias. The lockfile entries are removed by hand. Regenerating drops the supports-color peer annotations throughout, which is 2,176 lines of churn unrelated to this change.
9 tasks
lcottercertinia
marked this pull request as ready for review
September 18, 2026 18:55
lcottercertinia
approved these changes
Sep 18, 2026
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.
📝 PR Overview
Removes code the extension no longer runs, and gives the test suites shared setup.
Nothing a user can reach changes.
This is the first of two planned reduction PRs. This one is deletions and test-helper
extraction only, so it cannot alter behaviour. A second PR will share duplicated
production code — the three database grid views, the find and column-view controllers —
and is deliberately kept separate because those diffs touch live paths.
Draft: in progress. More commits are coming on this branch. Each one is a single
theme and lands with
pnpm lintandpnpm testgreen.🛠️ Changes made
Landed so far:
FlameChartuses
MeshRectangleRenderer,MeshMarkerRendererandMeshAxisRenderer, andSearchOrchestratorusesMeshSearchStyleRenderer. No production file imported thesprite path. Deletes
AxisRenderer(287),SearchStyleRenderer(209),TimelineMarkerRenderer(192),SpritePool(154),EventBatchRenderer(144) — whoseonly importers were each other — plus
markers.test.ts(699) andbatching.test.ts(564), which constructed only those classes.
Still to come on this branch:
TimeGridCalculator.ts, which has no importer.@rollup/plugin-aliasdevDependency.#vscode-elementsin the jest config, removing 35 copies of the samejest.mock(…)line.mount/settlepair (28 local copies today), log eventbuilders (4 near-identical
createEventclones), apex-log fixtures (the sameEXECUTION_STARTEDline pasted across 22 files), tabulator and pixi doubles, and thelana
asContextcast (67 occurrences).🧩 Type of change (check all applicable)
📷 Screenshots / gifs / video [optional]
No UI change. The timeline renders through the mesh path before and after.
🔗 Related Issues
None.
✅ Tests added?
sortMarkersByTimeAndSeverityhad no coverage outside the deletedmarkers.test.ts, soit gains a case in
MarkerProcessor.test.ts. The later commits add shared helpers ratherthan new cases.
Coverage this PR gives up.
markers.test.tsalso covered marker viewport culling,end-time resolution and per-type colour. Those cases asserted sprite
tint,alpha,xand
width— internals of the deleted class — so they could not move across as written.MeshMarkerRendererkeepsvisibleIndicatorsprivate and had no suite of its own beforethis PR either, so the gap is pre-existing rather than new, but it is not closed here.
The fix worth making is extracting
MeshMarkerRenderer.render()'s pure per-marker blockinto
MarkerProcessor, where those three behaviours become directly testable. Filed asfollow-up, not done here.
batching.test.tsneeds no replacement:TemporalSegmentTree.test.tsandRectangleCache.bucket.test.tsalready cover the culling, size classification andbucketing it exercised, against the live path.
📚 Docs updated?
No user-visible change, so no CHANGELOG entry. README and help site untouched.
Anything else we need to know? [optional]
Verified per commit with
pnpm test,pnpm lintandpnpm build. The build is theload-bearing check for the deletions — rollup has to resolve every import for the bundles
to be produced.
QuickPickWorkspace.tshas no caller and is kept on purpose: it holds themulti-root workspace prompt we want to reinstate. It now carries a comment saying so, and
naming
RetrieveLogFile.ts:68andLogView.ts:213as the two sites that takeworkspaceFolders[0]today.Three things found while reviewing, all left alone as out of scope:
MetricStripRenderer.ts:471re-sorts the marker array every frame(
[...markers].sort) inside the 16.6ms budget, and exception markers are unbounded.Memoising on array identity is a three-line fix.
SearchHighlightRenderer.ts:150hardcodesconst EVENT_HEIGHT = 15where its twinuses
TIMELINE_CONSTANTS.EVENT_HEIGHT, which that file already imports.halfGap,gappedHeight) is repeated across five sites,and the mesh ones clamp with
Math.max(0, …)whereHighlightRendererdoes not. The"must match" comments describe that coupling instead of enforcing it.
Out of scope by agreement:
apex-log-parser/is untouched, and the legacy timelinebehind
lana.timeline.legacyis a separate piece of work.