Skip to content

Commit f592522

Browse files
authored
Merge pull request #299 from pathsim/fix/toolbox-dependency-resolution
Resolve toolbox dependencies by source identity and for component imports
2 parents f9bb1c5 + 2c156b7 commit f592522

7 files changed

Lines changed: 120 additions & 13 deletions

File tree

src/lib/components/dialogs/ToolboxManagerDialog.svelte

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
upsertToolbox,
1414
removeToolbox,
1515
toolboxes,
16+
toolboxSourceKey,
1617
type CatalogEntry,
1718
type ToolboxConfig,
1819
type ToolboxSource,
@@ -162,7 +163,7 @@
162163
resolvedImportPath = (pypiImportPath.trim() || pkg).replace(/-/g, '_');
163164
resolvedDisplayName = displayNameInput.trim() || pkg;
164165
resolvedEventsImportPath = eventsImportPathInput.trim() || undefined;
165-
toolboxId = `pypi:${pkg}`;
166+
toolboxId = toolboxSourceKey(resolvedSource);
166167
categoryByClass = {};
167168
defaultCategory = undefined;
168169
step = 'trust';
@@ -174,7 +175,7 @@
174175
resolvedImportPath = urlImportPath.trim();
175176
resolvedDisplayName = displayNameInput.trim() || urlImportPath.trim();
176177
resolvedEventsImportPath = eventsImportPathInput.trim() || undefined;
177-
toolboxId = `url:${urlValue.trim()}`;
178+
toolboxId = toolboxSourceKey(resolvedSource);
178179
categoryByClass = {};
179180
defaultCategory = undefined;
180181
step = 'trust';
@@ -194,7 +195,9 @@
194195
resolvedImportPath = '';
195196
resolvedDisplayName = displayNameInput.trim() || fileName.replace(/\.py$/, '');
196197
resolvedEventsImportPath = undefined;
197-
toolboxId = `inline:${fileName}`;
198+
// Content-addressed: keying the id on the code (not the filename)
199+
// keeps two different uploads with the same name distinct.
200+
toolboxId = toolboxSourceKey(resolvedSource);
198201
categoryByClass = {};
199202
defaultCategory = undefined;
200203
step = 'trust';
@@ -326,10 +329,12 @@
326329
onClose();
327330
}
328331
329-
// Catalog entries that aren't already installed
332+
// Catalog entries that aren't already installed. Matched on source
333+
// identity, not id, so a catalog entry installed via the PyPI tab still
334+
// counts as installed.
330335
const availableCatalog = $derived.by(() => {
331-
const installedIds = new Set(installed.map((t) => t.id));
332-
return TOOLBOX_CATALOG.filter((e) => !installedIds.has(e.id));
336+
const installedKeys = new Set(installed.map((t) => toolboxSourceKey(t.source)));
337+
return TOOLBOX_CATALOG.filter((e) => !installedKeys.has(toolboxSourceKey(e.source)));
333338
});
334339
335340
// Dot index across the add-toolbox flow (manager view = no progress dots)

src/lib/schema/componentOps.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { COMPONENT_VERSION } from '$lib/types/component';
1111
import { graphStore } from '$lib/stores/graph';
1212
import { NODE_TYPES } from '$lib/constants/nodeTypes';
1313
import { downloadJson } from '$lib/utils/download';
14+
import { collectRequiredToolboxes } from '$lib/toolbox';
1415
import { cleanNodeForExport } from './cleanParams';
1516
import { hasFileSystemAccess } from './fileOps';
1617

@@ -25,6 +26,10 @@ export function createBlockFile(node: NodeInstance): ComponentFile {
2526
// Remove graph property for blocks (only subsystems have graphs)
2627
delete cleanedNode.graph;
2728

29+
// Record the toolbox this block needs (empty for builtin blocks) so a
30+
// direct .blk import can resolve the dependency instead of hard-failing.
31+
const requiredToolboxes = collectRequiredToolboxes([cleanedNode]);
32+
2833
return {
2934
version: COMPONENT_VERSION,
3035
type: 'block',
@@ -34,7 +39,8 @@ export function createBlockFile(node: NodeInstance): ComponentFile {
3439
modified: new Date().toISOString()
3540
},
3641
content: {
37-
node: cleanedNode
42+
node: cleanedNode,
43+
...(requiredToolboxes.length > 0 ? { requiredToolboxes } : {})
3844
} as BlockContent
3945
};
4046
}
@@ -51,6 +57,10 @@ export function createSubsystemFile(node: NodeInstance): ComponentFile {
5157
const clonedNode = structuredClone(node);
5258
const cleanedNode = cleanNodeForExport(clonedNode);
5359

60+
// Walk the nested graph for any toolbox blocks (collectRequiredToolboxes
61+
// recurses into subsystem graphs) so a direct .sub import can resolve them.
62+
const requiredToolboxes = collectRequiredToolboxes([cleanedNode]);
63+
5464
return {
5565
version: COMPONENT_VERSION,
5666
type: 'subsystem',
@@ -60,7 +70,8 @@ export function createSubsystemFile(node: NodeInstance): ComponentFile {
6070
modified: new Date().toISOString()
6171
},
6272
content: {
63-
node: cleanedNode
73+
node: cleanedNode,
74+
...(requiredToolboxes.length > 0 ? { requiredToolboxes } : {})
6475
} as SubsystemContent
6576
};
6677
}

src/lib/schema/fileOps.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -643,9 +643,20 @@ export function validateNodeTypes(nodes: NodeInstance[]): string[] {
643643
* Import a block or subsystem component at the given position
644644
* Uses shared cloneNodeForPaste utility for consistent ID regeneration
645645
*/
646-
function importComponent(content: BlockContent | SubsystemContent, position: Position): string[] {
646+
async function importComponent(
647+
content: BlockContent | SubsystemContent,
648+
position: Position
649+
): Promise<string[]> {
647650
const node = content.node;
648651

652+
// Install any runtime toolboxes the component declared before validating
653+
// node types — without this, a .blk/.sub using a toolbox block would
654+
// hard-fail even though the file records what it needs. Best-effort:
655+
// a skipped/failed install just falls through to the unknown-type error.
656+
if (content.requiredToolboxes && content.requiredToolboxes.length > 0) {
657+
await installRequiredToolboxes(content.requiredToolboxes);
658+
}
659+
649660
// Validate all node types are registered (recursive for subsystems)
650661
const invalidTypes = validateNodeTypes([node]);
651662
if (invalidTypes.length > 0) {
@@ -733,7 +744,10 @@ async function processImportContent(
733744
case 'block':
734745
case 'subsystem': {
735746
const position = options.position || { x: 100, y: 100 };
736-
const nodeIds = importComponent(componentFile.content as BlockContent | SubsystemContent, position);
747+
const nodeIds = await importComponent(
748+
componentFile.content as BlockContent | SubsystemContent,
749+
position
750+
);
737751
return { success: true, type: componentFile.type, nodeIds };
738752
}
739753

src/lib/toolbox/dependencies.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { NODE_TYPES } from '$lib/constants/nodeTypes';
1414
import type { NodeInstance } from '$lib/types/nodes';
1515
import type { ToolboxRequirement } from '$lib/types/schema';
1616
import { toolboxes } from './store';
17+
import { toolboxSourceKey } from './identity';
1718
import type { ToolboxConfig } from './types';
1819

1920
function walkNodeTypes(nodes: NodeInstance[], out: Set<string>): void {
@@ -62,9 +63,16 @@ export function collectRequiredToolboxes(nodes: NodeInstance[]): ToolboxRequirem
6263
/**
6364
* Filter a list of toolbox requirements to those that are NOT currently
6465
* installed. Used at load time to figure out what to prompt for.
66+
*
67+
* Matching is done on the source content identity (`toolboxSourceKey`), not
68+
* the raw `id`: the id depends on how a toolbox was added (catalog vs PyPI
69+
* tab vs file upload), so the same package can carry different ids across
70+
* machines. Comparing source keys means a file that references
71+
* `pypi:pathsim-chem` resolves against a catalog-installed `pathsim-chem`,
72+
* and an inline toolbox is matched by its code rather than its filename.
6573
*/
6674
export function findMissingRequirements(reqs: ToolboxRequirement[]): ToolboxRequirement[] {
6775
if (!reqs || reqs.length === 0) return [];
68-
const installed = new Set(get(toolboxes).map((t) => t.id));
69-
return reqs.filter((r) => !installed.has(r.id));
76+
const installedKeys = new Set(get(toolboxes).map((t) => toolboxSourceKey(t.source)));
77+
return reqs.filter((r) => !installedKeys.has(toolboxSourceKey(r.source)));
7078
}

src/lib/toolbox/identity.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* Content identity for runtime toolboxes.
3+
*
4+
* A toolbox `id` is a UI-construction artifact: catalog entries have a fixed
5+
* id, the PyPI tab builds `pypi:<pkg>`, the URL tab `url:<url>`, the file
6+
* upload `inline:<...>`. The id therefore depends on *how* the user added a
7+
* toolbox, not on *what* it is. Resolving dependencies on the raw id breaks
8+
* in two directions:
9+
*
10+
* - the same PyPI package added via the catalog vs the PyPI tab gets two
11+
* different ids and counts as two separate toolboxes;
12+
* - two different uploaded `.py` files that happen to share a filename get
13+
* the same id and count as one.
14+
*
15+
* `toolboxSourceKey` derives a stable key purely from the install source, so
16+
* both sides of a comparison agree regardless of the id.
17+
*/
18+
19+
import type { ToolboxSource } from './types';
20+
21+
/**
22+
* Small, stable string hash (djb2 variant). Used to content-address inline
23+
* toolbox sources — not security-sensitive, just deterministic and
24+
* collision-resistant enough to tell pasted Python files apart.
25+
*/
26+
export function hashString(s: string): string {
27+
let h = 5381;
28+
for (let i = 0; i < s.length; i++) {
29+
h = (h * 33) ^ s.charCodeAt(i);
30+
}
31+
return (h >>> 0).toString(36);
32+
}
33+
34+
/**
35+
* Normalize a PyPI project name per PEP 503: lowercase, with runs of `-`,
36+
* `_` and `.` collapsed to a single `-`. `Pathsim_Chem` and `pathsim-chem`
37+
* resolve to the same project.
38+
*/
39+
function normalizePypiName(pkg: string): string {
40+
return pkg.trim().toLowerCase().replace(/[-_.]+/g, '-');
41+
}
42+
43+
/**
44+
* Canonical content identity for a toolbox source. Two sources that install
45+
* the same toolbox produce the same key; two that don't, don't.
46+
*
47+
* Note: the PyPI key intentionally ignores `version` — a pinned and an
48+
* unpinned install of the same package are still the same toolbox for
49+
* "is it installed" purposes.
50+
*/
51+
export function toolboxSourceKey(source: ToolboxSource): string {
52+
if (!source || typeof source !== 'object') return 'unknown:';
53+
switch (source.type) {
54+
case 'pypi':
55+
return `pypi:${normalizePypiName(source.pkg)}`;
56+
case 'url':
57+
return `url:${source.url.trim()}`;
58+
case 'inline':
59+
return `inline:${hashString(source.code)}`;
60+
default:
61+
return `unknown:${JSON.stringify(source)}`;
62+
}
63+
}

src/lib/toolbox/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,5 @@ export { installAndRegisterToolbox, type InstallSpec } from './installFlow';
3939
export { seedPreloadedToolboxes } from './store';
4040

4141
export { collectRequiredToolboxes, findMissingRequirements } from './dependencies';
42+
43+
export { toolboxSourceKey, hashString } from './identity';

src/lib/types/component.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import type { NodeInstance } from './nodes';
6-
import type { GraphContent } from './schema';
6+
import type { GraphContent, ToolboxRequirement } from './schema';
77

88
/** Component types that can be saved/loaded */
99
export type ComponentType = 'block' | 'subsystem' | 'model';
@@ -24,11 +24,15 @@ export interface ComponentFile {
2424
/** Single block (no connections) */
2525
export interface BlockContent {
2626
node: NodeInstance;
27+
/** Runtime toolboxes this block needs (absent for builtin blocks). */
28+
requiredToolboxes?: ToolboxRequirement[];
2729
}
2830

2931
/** Subsystem (nested graph) */
3032
export interface SubsystemContent {
3133
node: NodeInstance; // The subsystem node (includes .graph)
34+
/** Runtime toolboxes the subsystem's blocks need (absent if all builtin). */
35+
requiredToolboxes?: ToolboxRequirement[];
3236
}
3337

3438
/** Full model - uses shared GraphContent structure */

0 commit comments

Comments
 (0)