Skip to content

refactor: consolidate manifest resolution and migrate console.* to logger#561

Merged
rgbkrk merged 2 commits intomainfrom
refactor/consolidate-manifest-resolution-and-logger-usage
Mar 6, 2026
Merged

refactor: consolidate manifest resolution and migrate console.* to logger#561
rgbkrk merged 2 commits intomainfrom
refactor/consolidate-manifest-resolution-and-logger-usage

Conversation

@rgbkrk
Copy link
Member

@rgbkrk rgbkrk commented Mar 6, 2026

Two cleanup items from a codebase audit.

Manifest resolution deduplicationmaterialize-cells.ts and useManifestResolver.ts had ~150 LOC of duplicated types and blob resolution logic. Extracted into a shared lib/manifest-resolution.ts module. This also removes an unsafe as Record<string, unknown> type cast for display_id — the shared resolveManifest handles it directly via the existing JupyterOutput type.

Logger migration — 7 files were using raw console.* calls, bypassing the logger abstraction that gates debug output in production. Migrated all of them: useCondaDependencies, useDependencies, useDenoDependencies, useGitInfo, useTrust, useUpdater, MarkdownCell.

Net: -135 lines, one fewer copy-paste surface, consistent logging.

rgbkrk added 2 commits March 6, 2026 06:55
…gger

Extract shared manifest resolution types (ContentRef, OutputManifest) and
functions (isManifestHash, resolveContentRef, resolveDataBundle,
resolveManifest) into lib/manifest-resolution.ts, eliminating ~150 LOC of
duplication between materialize-cells.ts and useManifestResolver.ts.

This also removes an unsafe `as Record<string, unknown>` type cast that
was used to set display_id in useManifestResolver.ts — the shared
resolveManifest now handles display_id directly via the JupyterOutput type
which already supports the field.

Migrate all raw console.* calls to the logger abstraction across 7 files:
useCondaDependencies, useDependencies, useDenoDependencies, useGitInfo,
useTrust, useUpdater, MarkdownCell. The logger gates debug output in
production builds.
The @/ alias maps to desktop/src/, not apps/notebook/src/. The logger
module lives in the notebook app, so it needs a relative import path.
@rgbkrk rgbkrk force-pushed the refactor/consolidate-manifest-resolution-and-logger-usage branch from 3805a23 to a2df0ac Compare March 6, 2026 14:55
@rgbkrk rgbkrk merged commit 145b47b into main Mar 6, 2026
10 checks passed
@rgbkrk rgbkrk deleted the refactor/consolidate-manifest-resolution-and-logger-usage branch March 6, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant