Skip to content

feat(utils): add n-ary tree LCA utility#1716

Closed
nikhilachale wants to merge 3 commits into
mainfrom
feat/nary-lca-utility
Closed

feat(utils): add n-ary tree LCA utility#1716
nikhilachale wants to merge 3 commits into
mainfrom
feat/nary-lca-utility

Conversation

@nikhilachale
Copy link
Copy Markdown
Collaborator

Summary

  • Adds the @composio/ao-utils workspace package with a typed n-ary tree LCA utility.
  • Keeps the utils package covered by Vitest tests.
  • Repairs the rebased pnpm lockfile resolution so frozen installs pass.
  • Stabilizes core plugin integration tests by explicitly aliasing core package imports when plugin source files are loaded by core Vitest.

Validation

  • pnpm install --frozen-lockfile
  • pnpm build
  • pnpm typecheck
  • pnpm lint
  • pnpm test

Follow-up/replacement for #1712 and #1713.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Test Coverage Report

Changed files have no coverage data (not instrumented or no tests ran).

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Greptile Summary

This PR introduces the @composio/ao-utils workspace package containing a typed n-ary tree LCA utility, adds a Vitest test suite for it, fixes the pnpm lockfile for frozen installs, and adds resolve.alias entries in packages/core/vitest.config.ts to redirect @aoagents/ao-core sub-path imports back to TypeScript source when plugin files are loaded during integration tests.

  • packages/utils/src/nary-lca.ts: Post-order DFS implementation that correctly identifies the LCA for unique-value trees; the found1/found2 closure flags ensure nodes absent from the tree produce a null result, and the ancestor-of-itself case is handled by returning the target node before checking hits.
  • packages/core/vitest.config.ts: Top-level resolve.alias array (regex-based) added alongside the existing test.alias record to ensure secondary @aoagents/ao-core imports from plugin source files resolve to local TypeScript rather than a stale built package.
  • packages/utils: New package scaffolding (tsconfig, vitest config, package.json) is minimal and consistent with the rest of the monorepo; the package name scope and Vitest version discrepancies noted in prior review threads remain open.

Confidence Score: 5/5

Safe to merge — new utility package is self-contained, the LCA algorithm is correct for the documented use case, and the core Vitest alias fix is a targeted, low-risk change.

The LCA implementation is logically sound for unique-value trees, the test suite covers the documented edge cases, and the core config change only affects test-time module resolution. No changes touch runtime session logic or any shared critical path.

No files require special attention beyond the package naming and Vitest version questions already discussed in prior threads.

Important Files Changed

Filename Overview
packages/utils/src/nary-lca.ts New post-order DFS LCA implementation; correct for unique-value trees, but hits.length >= 2 heuristic silently misbehaves when duplicate node values are present (already flagged in prior thread)
packages/utils/src/tests/nary-lca.test.ts Good coverage of common cases (ancestors, roots, nulls, single-node), though no test for val1===val2 when both are absent from the tree (code handles it correctly, just untested)
packages/core/vitest.config.ts Adds top-level resolve.alias for @aoagents/ao-core sub-paths so plugin source files loaded by integration tests can resolve core imports back to TypeScript source without needing circular devDeps
packages/utils/package.json New workspace package; uses @composio/ scope (inconsistent with @aoagents/ convention) and vitest ^4.0.18 (consistent with core but not plugins — both concerns flagged in prior thread)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["findLCA(root, val1, val2)"] --> B["dfs(node)"]
    B --> C{node === null?}
    C -- yes --> D["return null"]
    C -- no --> E["recurse into all children\ncollect non-null results in hits[]"]
    E --> F{node.val === val1\nor val2?}
    F -- yes --> G["set found1/found2\nreturn node\n(self is LCA or signal)"]
    F -- no --> H{hits.length >= 2?}
    H -- yes --> I["return node\n(LCA: two targets\nin separate subtrees)"]
    H -- no --> J["return hits[0] ?? null\n(propagate single found node)"]
    G --> K["candidate = dfs(root)"]
    I --> K
    J --> K
    D --> K
    K --> L{found1 && found2?}
    L -- no --> M["return null\n(one or both targets absent)"]
    L -- yes --> N["return candidate"]
Loading

Reviews (3): Last reviewed commit: "test: stabilize core plugin integration ..." | Re-trigger Greptile

Comment thread packages/utils/package.json
Comment thread packages/utils/package.json
Comment thread packages/utils/src/nary-lca.ts
@nikhilachale nikhilachale force-pushed the feat/nary-lca-utility branch 2 times, most recently from eea06f0 to c6eddb0 Compare May 7, 2026 20:50
Pritom14 and others added 3 commits May 8, 2026 02:38
Adds @composio/ao-utils package with findLCA() — post-order DFS
implementation that correctly handles ancestor-is-LCA, same-node,
and missing-value cases. All 11 tests pass.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@nikhilachale nikhilachale force-pushed the feat/nary-lca-utility branch from c6eddb0 to b5e3cd8 Compare May 7, 2026 21:12
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.

2 participants