From 35b7d8da09524872ee7f6519ffff5366495fbc5d Mon Sep 17 00:00:00 2001 From: Akshay-2007-1 Date: Wed, 22 Jul 2026 16:46:10 +0800 Subject: [PATCH] fix: accept DataType.ARRAY as equally valid to PAIR for tree nodes py-slang's module interface no longer has a distinct "pair" representation (source-academy/py-slang#307): pythonToModule now builds every Python list as a flat DataType.ARRAY, never a DataType.PAIR/EMPTY_LIST chain. A tree built here via make_tree/pair_make and round-tripped out to Python (via moduleToPython) then passed back into entry()/left_branch()/right_branch() now arrives tagged ARRAY, not PAIR - is_tree/assertNonEmptyTree hardcoded `value.type !== DataType.PAIR` checks that read conductor's DataType tag directly, so this broke without a change here. Fixed by accepting either DataType.PAIR or DataType.ARRAY wherever a tree node's own tag is checked (isPairLike) - pair_head/pair_tail already read either shape identically (that's py-slang's own GenericDataHandler bridge), so this is purely about the validation, not the traversal. make_tree's own construction (pair_make calls) is unchanged - it always freshly builds a genuine PAIR for a node it creates itself; only validation of an incoming (possibly round-tripped) value needed to widen. NOTE: written and typechecked by inspection only - this worktree (feat/migrate-binary-tree) has no node_modules installed, and borrowing another worktree's (cross-branch, version-drifted @sourceacademy/conductor copies) produced unrelated pre-existing type errors in the test file that made a real yarn install/tsc/test run unreliable to interpret here. Needs a real `yarn install && yarn tsc && yarn test` pass in this bundle before merging. --- .../binary_tree/src/__tests__/index.test.ts | 20 +++++++++++ src/bundles/binary_tree/src/functions.ts | 35 +++++++++++++------ 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/bundles/binary_tree/src/__tests__/index.test.ts b/src/bundles/binary_tree/src/__tests__/index.test.ts index 48ac459e44..5a73b000e3 100644 --- a/src/bundles/binary_tree/src/__tests__/index.test.ts +++ b/src/bundles/binary_tree/src/__tests__/index.test.ts @@ -62,6 +62,26 @@ describe(funcs.is_tree, () => { ); await expect(funcs.is_tree(handler, tree)).resolves.toEqual(true); }); + + it('returns true for a tree round-tripped back in as DataType.ARRAY, not just DataType.PAIR', async () => { + // Per py-slang: pythonToModule now builds every Python list as a flat DataType.ARRAY, never a + // DataType.PAIR chain - so a tree round-tripped out to Python (via moduleToPython) and passed + // back into e.g. entry()/left_branch() arrives tagged ARRAY, not PAIR. is_tree (and + // assertNonEmptyTree) must accept either shape - this is the regression test for that. + const handler = new TestDataHandler(); + const rightPair = await handler.array_make(DataType.ANY, 2, emptyListValue()); + await handler.array_set(rightPair, 0, emptyListValue()); + await handler.array_set(rightPair, 1, emptyListValue()); + const leftPair = await handler.array_make(DataType.ANY, 2, emptyListValue()); + await handler.array_set(leftPair, 0, emptyListValue()); + await handler.array_set(leftPair, 1, rightPair); + const tree = await handler.array_make(DataType.ANY, 2, emptyListValue()); + await handler.array_set(tree, 0, await opaqueNumber(handler, 0)); + await handler.array_set(tree, 1, leftPair); + + await expect(funcs.is_tree(handler, tree)).resolves.toEqual(true); + await expect(handler.opaque_get(await funcs.entry(handler, tree))).resolves.toEqual(0); + }); }); describe(funcs.make_tree, () => { diff --git a/src/bundles/binary_tree/src/functions.ts b/src/bundles/binary_tree/src/functions.ts index 87ad742e9d..90029e6758 100644 --- a/src/bundles/binary_tree/src/functions.ts +++ b/src/bundles/binary_tree/src/functions.ts @@ -61,24 +61,36 @@ export async function make_tree( export async function is_tree(evaluator: IDataHandler, value: TypedValue): Promise { if (!value) return false; if (value.type === DataType.EMPTY_LIST) return true; - if (value.type !== DataType.PAIR) return false; + // A tree node is a pair - per conductor's "a pair is just an array of length 2" model, that may + // arrive tagged DataType.PAIR (built directly by make_tree) or DataType.ARRAY (round-tripped + // back in through a Python list, since py-slang's pythonToModule builds every list as an ARRAY + // now, not a PAIR chain). pair_head/pair_tail already read either shape the same way. + if (!isPairLike(value)) return false; - const rest = await evaluator.pair_tail(value); - if (rest.type !== DataType.PAIR) return false; + const rest = await evaluator.pair_tail(value as TypedValue); + if (!isPairLike(rest)) return false; - const left = await evaluator.pair_head(rest); + const left = await evaluator.pair_head(rest as TypedValue); if (!await is_tree(evaluator, left)) return false; - const rightRest = await evaluator.pair_tail(rest); - if (rightRest.type !== DataType.PAIR) return false; + const rightRest = await evaluator.pair_tail(rest as TypedValue); + if (!isPairLike(rightRest)) return false; - const right = await evaluator.pair_head(rightRest); + const right = await evaluator.pair_head(rightRest as TypedValue); if (!await is_tree(evaluator, right)) return false; - const tail = await evaluator.pair_tail(rightRest); + const tail = await evaluator.pair_tail(rightRest as TypedValue); return tail.type === DataType.EMPTY_LIST; } +/** A pair is just an array of length 2 (no distinct "pair" representation) - a tree node may be + * tagged either DataType.PAIR (built directly by make_tree's own pair_make calls) or + * DataType.ARRAY (round-tripped back in through Python, since pythonToModule builds every list as + * an ARRAY now). Both are equally valid; pair_head/pair_tail read either the same way. */ +function isPairLike(value: TypedValue): boolean { + return value.type === DataType.PAIR || value.type === DataType.ARRAY; +} + /** * Returns a boolean value, indicating whether the given * value is an empty binary tree. @@ -103,11 +115,14 @@ async function assertNonEmptyTree( throw new EvaluatorTypeError(`${funcName} expects binary tree`, 'binary tree', value ? DataType[value.type] : 'undefined'); } - if (value.type !== DataType.PAIR) { + if (!isPairLike(value)) { throw new EvaluatorRuntimeError(`${funcName} received an empty binary tree!`); } - return value; + // NonEmptyBinaryTree is declared DataType.PAIR, but the runtime value may genuinely be + // DataType.ARRAY (round-tripped back in through Python) - pair_head/pair_tail read either the + // same way (see isPairLike's doc comment), so this is a safe, documented cast, not a lie. + return value as NonEmptyBinaryTree; } /**