Fix `yarn install` warnings, update unit tests, and update libraries. #235 by swensel · Pull Request #236 · cognizant-ai-lab/neuro-san-ui

swensel · 2026-03-26T03:58:57Z

Description

'Grab bag' PR to follow up on lower priority items from previous PRs:

swensel · 2026-03-26T18:05:56Z

    beforeEach(() => {
        jest.resetAllMocks()
        jest.restoreAllMocks()
+        jest.useRealTimers()


Move jest.useRealTimers() to beforeEach. Was suggested in #223.

Any tests that break due to this, should break as it means they are making unholy assumptions.

It's a bit like the saying, "Anything that can be destroyed by the truth, should be destroyed by the truth."

swensel · 2026-03-26T18:19:37Z

        "@langchain/openai": "1.2.5",
        "@mui/icons-material": "7.3.1",
        "@mui/material": "7.3.1",
+        "@mui/system": "7.3.1",


Addressing some of the warnings on yarn install. They aren't eliminated entirely but here's the before and after.

Before

971999 neuro-san-ui grab-bag-PR $ yarn install
➤ YN0000: · Yarn 4.12.0
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed
➤ YN0000: ┌ Post-resolution validation
➤ YN0060: │ eslint is listed by your project with version 10.0.2 (p8b6f96), which doesn't satisfy what eslint-plugin-import and other dependencies request (but they have non-overlapping ranges!).
➤ YN0002: │ @cognizant-ai-lab/neuro-san-web@workspace:apps/main doesn't provide @mui/system (p35b046), requested by @cognizant-ai-lab/ui-common and other dependencies.
➤ YN0002: │ @cognizant-ai-lab/neuro-san-web@workspace:apps/main doesn't provide @types/react (p85dc55), requested by @emotion/react and other dependencies.
➤ YN0002: │ @cognizant-ai-lab/neuro-san-web@workspace:apps/main doesn't provide eslint (p594c2a), requested by eslint-config-next.
➤ YN0002: │ @cognizant-ai-lab/neuro-san-web@workspace:apps/main doesn't provide next (p50ffad), requested by @cognizant-ai-lab/ui-common and other dependencies.
➤ YN0002: │ @cognizant-ai-lab/neuro-san-web@workspace:apps/main doesn't provide react (p24db38), requested by @cognizant-ai-lab/ui-common and other dependencies.
➤ YN0002: │ @cognizant-ai-lab/neuro-san-web@workspace:apps/main doesn't provide react-dom (p671935), requested by @mui/material and other dependencies.
➤ YN0002: │ @cognizant-ai-lab/neuro-san-web@workspace:apps/main doesn't provide typescript (p4325d6), requested by @cognizant-ai-lab/ui-common and other dependencies.
➤ YN0002: │ @cognizant-ai-lab/ui-common@workspace:packages/ui-common doesn't provide react (p28b486), requested by @xyflow/react and other dependencies.
➤ YN0002: │ @cognizant-ai-lab/ui-common@workspace:packages/ui-common doesn't provide typescript (pdc71d9), requested by openapi-typescript.
➤ YN0086: │ Some peer dependencies are incorrectly met by your project; run yarn explain peer-requirements for details, where is the six-letter p-prefixed code.
➤ YN0086: │ Some peer dependencies are incorrectly met by dependencies; run yarn explain peer-requirements for details.
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed in 0s 423ms
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed
➤ YN0000: · Done with warnings in 0s 656ms

After

971999 neuro-san-ui grab-bag-PR $ yarn install
➤ YN0000: · Yarn 4.12.0
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed
➤ YN0000: ┌ Post-resolution validation
➤ YN0060: │ eslint is listed by your project with version 10.0.2 (p8b6f96), which doesn't satisfy what eslint-plugin-import and other dependencies request (but they have non-overlapping ranges!).
➤ YN0002: │ @cognizant-ai-lab/neuro-san-web@workspace:apps/main doesn't provide eslint (p594c2a), requested by eslint-config-next.
➤ YN0086: │ Some peer dependencies are incorrectly met by your project; run yarn explain peer-requirements for details, where is the six-letter p-prefixed code.
➤ YN0086: │ Some peer dependencies are incorrectly met by dependencies; run yarn explain peer-requirements for details.
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed in 0s 418ms
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed
➤ YN0000: · Done with warnings in 0s 654ms

I took another look at this.

Here is the new after

971999 neuro-san-ui grab-bag-PR $ yarn install
➤ YN0000: · Yarn 4.12.0
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed
➤ YN0000: ┌ Post-resolution validation
➤ YN0060: │ eslint is listed by your project with version 10.0.2 (p594c2a), which doesn't satisfy what eslint-plugin-import (via eslint-config-next) and other dependencies request (^9.7.0).
➤ YN0060: │ eslint is listed by your project with version 10.0.2 (p8b6f96), which doesn't satisfy what eslint-plugin-import and other dependencies request (but they have non-overlapping ranges!).
➤ YN0086: │ Some peer dependencies are incorrectly met by your project; run yarn explain peer-requirements for details, where is the six-letter p-prefixed code.
➤ YN0086: │ Some peer dependencies are incorrectly met by dependencies; run yarn explain peer-requirements for details.
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed in 0s 407ms
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed
➤ YN0000: · Done with warnings in 0s 637ms

These are both expected since eslint-plugin-import doesn't specify eslint v10 yet, but otherwise it appears to work fine in our setup:

➤ YN0060: │ eslint is listed by your project with version 10.0.2 (p594c2a), which doesn't satisfy what eslint-plugin-import (via eslint-config-next) and other dependencies request (^9.7.0). ➤ YN0060: │ eslint is listed by your project with version 10.0.2 (p8b6f96), which doesn't satisfy what eslint-plugin-import and other dependencies request (but they have non-overlapping ranges!).

That will eventually be resolved once eslint-plugin-import releases a new version. I did see there is this issue in their GitHub for tracking the eslint upgrade.

These seem to be generic warnings that are shown if any peer dependency warnings are shown:

➤ YN0086: │ Some peer dependencies are incorrectly met by your project; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code. ➤ YN0086: │ Some peer dependencies are incorrectly met by dependencies; run yarn explain peer-requirements for details.

In the end I went with what I consider to be the right change to make, rather than just suppressing warnings. This applies to all of the package.json changes in this PR.

swensel · 2026-03-26T18:42:10Z

        "jest-silent-reporter",

+        // Declared at root for visibility/version governance; used by apps/main
+        "next",


Without this, knip was complaining about next being an unused dependency. It was suggested to move next to the root package.json in #233.

You suggested to move this to the root package.json (and that made sense to me), but when I did, knip complained, so I needed to add this here.

so I needed to add this here

Again, this "makes the angry red squigglies go away", but is it right? Why is knip complaining? It's not impossible for knip to have false positives but very very rare. Is it telling us something here?

I ended up removing next from the root package.json. I don't think it should be there. It is appropriate to have it in apps/main/package.json, since it's used there. Based on that it this no longer needs to be in knip.config.ts.

swensel · 2026-03-26T18:44:12Z

-    afterEach(() => {
-        // Restore real timers after every test so that any test calling jest.useFakeTimers() cannot leak fake timers
-        // into subsequent tests.
-        jest.useRealTimers()


Same as this comment. Applies to the other instances where I moved jest.useRealTimers() as well.

swensel · 2026-03-26T18:46:42Z


+    // There is also a test in GraphLayouts.test.ts
+    // TODO: This doesn't seem like it would catch the display: none vs visibility: hidden issue we recently had.
+    it("Should render plasma edges between agents in conversation when isAwaitingLlm is true", () => {


I think this test makes sense to add. There are also these in GraphLayouts.test.ts:

describe("Plasma edges and known message types", () => { it.each([ {layoutFunction: layoutRadial, name: "radial"}, {layoutFunction: layoutLinear, name: "linear"}, ])("$name layout: creates plasma edge for all supported conversation types", ({layoutFunction}) => { .... it.each([ {layoutFunction: layoutRadial, name: "radial"}, {layoutFunction: layoutLinear, name: "linear"}, ])("$name layout: does NOT create plasma edge for excluded conversation type (HUMAN)", ({layoutFunction}) => { ....

Unfortunately, even with toBeVisible() used in this AgentFlow.test.tsx, it wouldn't have caught the issue that I fixed with #228. We have had some discussions on Slack about taking a different approach to testing, in terms of what the user really sees. We may need to depend on something like that in order to do this properly.

Are you saying there's no test we can write here that would catch that and similar issue?

I'm saying that I haven't been able to write a successful test yet that would catch the issue from #228 .

That is concerning.

I'm considering doing a test with Playwright to cover this case.

This was a quick test I did with an LLM on Playwright - #239. Seems to work fine, but, not for this PR.

swensel · 2026-03-26T18:47:15Z

    })

+    // There is also a test in GraphLayouts.test.ts
+    // TODO: This doesn't seem like it would catch the display: none vs visibility: hidden issue we recently had.


I can remove this TODO if we are OK keeping this test, otherwise, I'll remove the entire test case.

Seems like it would be trivial to verify whether it catches that issue or not?! Just reintroduce the issue and see if it fails? If I saw this comment in random code I'd think, "Why didn't they just try it?!

What I meant is that I did try reintroducing the issue, and this definitely did not catch it.

Ah, that's not how your comment was worded.

swensel · 2026-03-26T18:51:01Z

        await screen.findByTestId(expectedTestId)
    })

+    // Are these tests repeated somewhere? I don't see other code exercising the agentIconSuggestion prop?


@dsargent mentioned that these are repeated in #223. I don't see other code exercising the agentIconSuggestion prop? Am I missing something?

I might have been getting it confused with these tests:

neuro-san-ui/packages/ui-common/__tests__/components/Common/CustomerLogo.test.tsx

Line 38 in 6216e34

it("Falls back to supplied fallbackText logoSource is 'generic' but iconSuggestion is invalid", () => {

But the test here is testing different code so maybe still valuable. I think you said removing these reduces coverage, which is pretty telling.

Removing these tests does reduce coverage. I will remove this comment in the code though.

swensel · 2026-03-26T18:57:16Z

        "jest-fail-on-console": "3.3.1",
        "jest-silent-reporter": "0.6.0",
        "knip": "5.63.1",
+        "next": "16.2.1",


Moved to the root package.json.

Seems weird to have next as a devDep. But I admit I'm still a bit confused about the whole Workspaces thing and how it plays out with dependencies.

I thought the same thing. And actually, everything in the root package.json is a dev dep, including react, which also doesn't seem right. Perhaps we should split out the prod deps and dev deps there.

I think this was how Copilot advised setting it up, and I couldn't find any useful info elsewhere (StackOverflow etc.) so I went with that. But now we know a bit more about workspaces.

I did end up removing next from the root package.json as per this comment. The root only holds the workspace declaration and shared tooling, so it makes sense that everything goes into devDependencies there. The Yarn docs also say this about dev deps:

Set of dependencies that must be made available to the current package in order for it to work properly as a workspace.

Unlike regular dependencies, those listed in devDependencies will only be required when the package is installed as part of a workspace project - usually by cloning the project repository then running yarn install inside it.

swensel · 2026-03-26T19:01:52Z

        "rehype-raw": "7.0.0",
        "rehype-slug": "6.0.0",
-        "zustand": "4.4.7"
+        "zustand": "4.5.7"


More changes to reduce yarn install warnings.

Again, target fixation. I appreciate the attempt to reduce warnings, but is it right?!

Wrong: "What can I shuffle around to placate the volcano god and make the warnings go away?" ❌
Right: "What is this warning telling me? What is the right way to fix it?" ✅

Do you see the difference in mindset?

Yes, I think I need to expand my mindset to not just reduce warnings or to up test coverage, but to consider the right way, and deeper intention, behind doing either of those things.

As a fellow martial arts fan, a Bruce Lee simile is appropriate:

“It's like a finger pointing away to the moon. Don't concentrate on the finger or you will miss all that heavenly glory.”

Here, the "moon" is: better code, better ways of doing things. The finger pointing to is: test coverage numbers, yarn/lint warnings, ...

This was helpful. I looked at this PR today without so much target fixation and came to better conclusions and IMO a better outcome.

Copilot

Pull request overview

Follow-up “grab bag” maintenance PR focused on dependency alignment in the monorepo and test-suite hygiene (especially around Jest timers), plus a small addition of UI behavior coverage.

Changes:

Updated several dev tooling/runtime dependencies (e.g., eslint-plugin-jest, eslint-plugin-testing-library, zustand), and adjusted workspace dependency declarations (including adding next at the root and as a ui-common peer dependency).
Centralized Jest timer cleanup by moving jest.useRealTimers() into the shared withStrictMocks() test helper and removing redundant per-file timer resets.
Added/adjusted a few tests and comments (including a new AgentFlow plasma-edge assertion).

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`yarn.lock`	Lockfile updates reflecting dependency bumps and workspace dependency graph changes.
`package.json`	Root devDependency updates (notably adding `next`).
`knip.config.ts`	Ignore list updated to account for `next` being declared at the root.
`apps/main/package.json`	Adjusted app dependency declarations (added `@mui/system`, `react`, `react-dom`, TS typing/dev tooling entries).
`packages/ui-common/package.json`	Shifted `next` to a peer dependency and updated dev deps (e.g., `react`, `typescript`), plus `zustand` bump.
`__tests__/common/strictMocks.ts`	Centralized timer reset (`jest.useRealTimers()`) in the shared strict-mocks helper.
`packages/ui-common/__tests__/components/**`	Removed redundant `afterEach(jest.useRealTimers)` blocks; added/adjusted a few assertions/comments and one new AgentFlow test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T19:32:53Z

        "@langchain/openai": "1.2.5",
        "@mui/icons-material": "7.3.1",
        "@mui/material": "7.3.1",
+        "@mui/system": "7.3.1",


Adding @mui/system as an app dependency pinned to 7.3.1 introduces two installed versions in the lockfile (7.3.1 and 7.3.9 via ^7.3.1). Mixing MUI package patch versions can lead to duplicated bundles and subtle runtime incompatibilities; align @mui/system to the same resolved version as the other @mui/* packages (or use the same version specifier) so Yarn can dedupe to a single version.

Suggested change

"@mui/system": "7.3.1",

"@mui/system": "^7.3.1",

This was added to reduce yarn install warnings. Also, we pin our deps.

"reducing warnings" is a bit blinkered -- are you sure this is the right thing to do? Just because it "makes the red squigglies go away" doesn't mean it's correct.

I'm not saying your changes to remove yarn warnings are right or wrong. I haven't investigated enough to say. But it sounds like you weren't focusing on that either, just whatever it takes to get the warnings to go away.

As I mentioned in other comments I took a fresh look at things today to make sure I had the right perspective.

Anyway, about this item, the right fix is to pin @mui/system to 7.3.9. This is the version that @mui/material ^7.3.1 already resolves to. With that, Yarn can dedupe to a single copy and the peer dep requirement is satisfied.

yarn why @mui/system confirms every requester lands on the same 7.3.9 so there is no duplication:

├─ @cognizant-ai-lab/neuro-san-web@workspace:apps/main │ └─ @mui/system@npm:7.3.9 (via npm:7.3.9) │ ├─ @mui/material@npm:7.3.1 │ └─ @mui/system@npm:7.3.9 (via npm:^7.3.1) │ └─ @mui/material@npm:7.3.1 [bf2e4] └─ @mui/system@npm:7.3.9 [bf2e4] (via npm:^7.3.1)

dsargent · 2026-03-26T20:19:21Z

General comment, the title is one of those that represents "developer's state of mind at one particular instance in time" and will not age well. The title should be something "eternal", summarizing what's in the PR, as an independent entity that can stand on its own. Imagine looking through a list of PRs a year from now and seeing this title...

That said, what you currently have as the title would be fine as context within the description, with a cross ref to "previous PRs".

dsargent

Please see comments

dsargent · 2026-03-26T20:21:13Z

    beforeEach(() => {
        jest.resetAllMocks()
        jest.restoreAllMocks()
+        jest.useRealTimers()


Any tests that break due to this, should break as it means they are making unholy assumptions.

It's a bit like the saying, "Anything that can be destroyed by the truth, should be destroyed by the truth."

dsargent · 2026-03-26T20:23:10Z

        "@langchain/openai": "1.2.5",
        "@mui/icons-material": "7.3.1",
        "@mui/material": "7.3.1",
+        "@mui/system": "7.3.1",


"reducing warnings" is a bit blinkered -- are you sure this is the right thing to do? Just because it "makes the red squigglies go away" doesn't mean it's correct.

dsargent · 2026-03-26T20:26:40Z

    })

+    // There is also a test in GraphLayouts.test.ts
+    // TODO: This doesn't seem like it would catch the display: none vs visibility: hidden issue we recently had.


Seems like it would be trivial to verify whether it catches that issue or not?! Just reintroduce the issue and see if it fails? If I saw this comment in random code I'd think, "Why didn't they just try it?!

dsargent · 2026-03-26T20:30:30Z


+    // There is also a test in GraphLayouts.test.ts
+    // TODO: This doesn't seem like it would catch the display: none vs visibility: hidden issue we recently had.
+    it("Should render plasma edges between agents in conversation when isAwaitingLlm is true", () => {


Are you saying there's no test we can write here that would catch that and similar issue?

dsargent · 2026-03-26T20:39:51Z

        "rehype-raw": "7.0.0",
        "rehype-slug": "6.0.0",
-        "zustand": "4.4.7"
+        "zustand": "4.5.7"


Again, target fixation. I appreciate the attempt to reduce warnings, but is it right?!

Wrong: "What can I shuffle around to placate the volcano god and make the warnings go away?" ❌
Right: "What is this warning telling me? What is the right way to fix it?" ✅

Do you see the difference in mindset?

dsargent · 2026-03-26T20:40:16Z

        "jest-silent-reporter",

+        // Declared at root for visibility/version governance; used by apps/main
+        "next",


dsargent · 2026-03-26T20:42:28Z

        "jest-fail-on-console": "3.3.1",
        "jest-silent-reporter": "0.6.0",
        "knip": "5.63.1",
+        "next": "16.2.1",


Seems weird to have next as a devDep. But I admit I'm still a bit confused about the whole Workspaces thing and how it plays out with dependencies.

dsargent · 2026-03-26T20:58:10Z

        await screen.findByTestId(expectedTestId)
    })

+    // Are these tests repeated somewhere? I don't see other code exercising the agentIconSuggestion prop?


I might have been getting it confused with these tests:

neuro-san-ui/packages/ui-common/__tests__/components/Common/CustomerLogo.test.tsx

Line 38 in 6216e34

it("Falls back to supplied fallbackText logoSource is 'generic' but iconSuggestion is invalid", () => {

But the test here is testing different code so maybe still valuable. I think you said removing these reduces coverage, which is pretty telling.

…common. Put eslint in dev deps in apps/main.

…r deeps from apps/main package.json, and upgrade mui/system there.

swensel · 2026-03-27T23:41:07Z

        "@langchain/openai": "1.2.5",
        "@mui/icons-material": "7.3.1",
        "@mui/material": "7.3.1",
+        "@mui/system": "7.3.9",


We need to bring our own version of @mui/system since it's a required peer dep of @mui/x-tree-view.

This comment is also relevant.

swensel · 2026-03-27T23:43:58Z

        "babel-jest",

+        // Declared in apps/main to satisfy eslint-config-next peer dep; lint itself runs from root
+        "eslint",


See comment. This admittedly was added by an LLM but I reasoned through with it and this seems correct to me.

Why would anything dev-related be a peer dep? I don't understand this.

Also, this will now make it so that if eslint is ever included as a dep where it's not actually needed, knip will be blind to it.

There are some other libraries already in this file though? Are those OK but not this one?

Every one should be (and has been) challenged. Disabling/Ignoring is a last resort. So now I'm challenging this one and "why now?" and "isn't there a better way rather than blindfolding knip?"

Another option could be to only disable it for apps/main in knip.config.ts:

workspaces: { "apps/main": { ignoreDependencies: [ // Declared to satisfy eslint-config-next peer dep; lint itself runs from root "eslint", // Declared to satisfy @mui/x-tree-view required peer dep; not directly imported "@mui/system", ], }, },

I'm also looking into if there's any other way around this.

So far I've just moved these to:

workspaces: { "apps/main": { ignoreDependencies: [ ...

I can look more into this but this PR has been open for a while.

Are you OK with this solution @dsargent ?

swensel · 2026-03-27T23:44:29Z


 const TEST_AGENT_MUSIC_NERD_PRO = "Music Nerd Pro"

+const mockPlasmaEdgeTestId = "mock-plasma-edge"


Move to consts for these data-testids.

…-view` but they do need to be included unless `styled-components` is included instead.

swensel · 2026-03-28T00:50:01Z

        "@cognizant-ai-lab/ui-common": "workspace:*",
-        "@emotion/react": "11.13.3",
-        "@emotion/styled": "11.13.0",
+        "@emotion/react": "11.14.0",


Upgrade @emotion/react and @emotion/styled to the latest versions.

Side note: @emotion/react and @emotion/styled are optional peer deps of @mui/x-tree-view but they do need to be included unless styled-components is included instead.

I guess they are "optional depending which styling library you're using, but if you're using emotion they are not optional".

swensel · 2026-03-28T01:35:31Z

+        "typescript": "5.9.3"
    },
    "peerDependencies": {
-        "@emotion/react": "^11.13.3",


Removed @emotion/react, @emotion/styled, and @mui/system from ui-common's peerDependencies. They are redundant. Yarn 4 propagates transitive peer requirements, so a consuming app already gets told about these by the packages that actually need them:

@emotion/react and @emotion/styled are required peers of @mui/material. Any app consuming ui-common has to install @mui/material anyway, and Yarn will tell them about @emotion from there.

@mui/system is a required peer of @mui/x-tree-view. Same thing as above, the app has to install @mui/x-tree-view, and Yarn propagates the @mui/system requirement automatically.

No need for ui-common to repeat constraints the consuming app is already going to see.

swensel · 2026-03-28T03:05:29Z

-        "@emotion/styled": "^11.13.0",
        "@mui/icons-material": "^7.3.1",
        "@mui/material": "^7.3.1",
-        "@mui/system": "^7.3.1",


I'm wondering why @mui/system and next-auth were added here? I was more looking at @mui/system in this PR, but I'm also not sure why we are including next-auth? Does smoother integration mean recommending a version for the end consumer?

next-auth is used for local testing, both in Neuro UI and MAUI.

If we don't make it a peer dep, you end up with two installations of next-auth in Neuro UI and next-auth doesn't like that.

@mui/system -- I don't remember but it fixed something.

Thanks, that makes sense for next-auth. I'll do a closer check for @mui/system.

Based on neuro-ui being fine, I don't think we need @mui/system in the peer deps here.

swensel · 2026-03-28T03:17:20Z

        "next": "16.2.1",
        "next-auth": "5.0.0-beta.30",
        "notistack": "3.0.2",
+        "react": "19.2.4",


These should be in the production deps not the peer deps.

swensel · 2026-03-28T03:19:58Z

-        "react-dom": "^19.2.4",
-        "typescript": "^5.9.2"
+        "node-mocks-http": "1.17.2",
+        "typescript": "5.9.3"


typescript should be in the dev deps in the app.

I'm still not sure about his. Don't we want to set a common Typesript version in the root package.json to make sure all packages in the workspace use the same version?

We need typescript in the dev deps here because ui-common declares typescript as a peer dep. Yarn 4 enforces peer dependencies at every level of the dependency tree, so the consuming package must explicitly provide it, otherwise we get YN0002 warnings.

Then what's the point of having it as a common dep under the root if each package still has to redeclare it? That doesn't sound right.

If I remove typescript at root I get ➤ YN0002: │ @cognizant-ai-lab/neuro-san-ui@workspace:. doesn't provide typescript (p84aa70), requested by @typescript-eslint/eslint-plugin and other dependencies.

That seems like a legit warning to me.

The LLMs say the right way is each package independently declares the thing they need, and use yarn constraints to ensure version consistency.

swensel · 2026-03-28T03:32:03Z

+        "react-dom": "19.2.4",
+        "typescript": "5.9.3"
    },
    "peerDependencies": {


@dsargent Is there a good way to test these changes on local with Neuro AI without publishing a new package? I know there is file:, but that seemed unreliable?

Yes.

In this repo:

yarn workspace @cognizant-ai-lab/ui-common pack

In Neuro UI:

yarn remove @cognizant-ai-lab/ui-common yarn add @cognizant-ai-lab/ui-common@file:../neuro-san-ui/packages/ui-common/package.tgz

That will give the true "npm repo" experience since it gets packaged and bundled exactly as in the real deployment case.

I think if you change neuro-san-ui you have to repeat the above steps.

This didn't work for me on the neuro-ui side. I'll reach out on Slack.

I figured it out. Running the command directly works fine (yarn workspace @cognizant-ai-lab/ui-common pack). I was trying to add yarn pack for convenience but that created the package.tgz at the root.

Anyway, this is the only new relevant warning in neuro-ui:

➤ YN0060: │ next is listed by your project with version 16.1.6 (pf1c754), which doesn't satisfy what @cognizant-ai-lab/ui-common and other dependencies request (^16.2.1).

We should upgrade next in neuro-ui anyway.

Tested this in Neuro UI now that Nick resolved the CORS issue. Looks fine.

swensel · 2026-03-28T03:35:42Z

        "@mui/material": "^7.3.1",
-        "@mui/system": "^7.3.1",
        "@mui/x-tree-view": "^8.22.0",
+        "next": "^16.2.1",


next makes more sense in the peer deps for the ui-common package since it's not directly used in this package, but it would be used by the consumers of this package (for now).

?! This package depends on the router from Next.js:

import {useRouter} from "next/router.js"

Also depends indirectly on Next.js via next-auth (dev dep though).

We used to use NextImage but I think I removed all those from ui-common.

True, we do have import {useRouter} from "next/router.js". I spoke too soon, I was searching for "next".

next is used by ui-common but the consumer should still provide it, as far as I can tell.

Which is exactly what peer deps are for.

dsargent · 2026-03-30T21:56:32Z

        // Used by Next.js image optimization,
        "sharp",

+        // Declared in apps/main to satisfy @mui/x-tree-view required peer dep; not directly imported


Same here, this seems drastic. Once something like this gets added to an "ignore" config, it tends to stay there forever, causing blind spots in the tooling.

There are some other libraries already here though?

See my previous comment on the same point.

swensel · 2026-04-02T02:57:56Z


 # Build stuff
 **/dist/
+package.tgz


Adding this to .gitignore.

swensel · 2026-04-02T02:58:43Z

        "packages/ui-common/components/MultiAgentAccelerator/const.ts",

+        // Used by another workspace (shared package-level exports)
+        "packages/ui-common/components/MultiAgentAccelerator/MultiAgentAccelerator.tsx",


I was getting these knip errors since these packages were used by another workspace:

971999 neuro-san-ui grab-bag-PR $ yarn knip Unused exports (2) MultiAgentAccelerator packages/ui-common/components/MultiAgentAccelerator/MultiAgentAccelerator.tsx:85:14 getTitleBase packages/ui-common/utils/title.ts:19:14 Unused exported types (1) NavbarProps interface packages/ui-common/components/Common/Navbar.tsx:49:18

dsargent

I think we're at the "good enough" stage (Probably way beyond)

Thanks for your patience with all the back and forth and discussions.

If you feel there are loose ends or things that still trouble you, please open a follow-up ticket.

swensel added 3 commits March 25, 2026 19:56

#223 follow up and add another plasma edge test in AgentFlow.test.tsx.

287364d

#233 follow up.

8de572a

#152 follow up - reducing the warnings on yarn install.

6f250c1

swensel changed the title ~~Grab bag' PR to follow up on lower priority items from previous PRs. #235~~ 'Grab bag' PR to follow up on lower priority items from previous PRs. #235 Mar 26, 2026

swensel added 2 commits March 25, 2026 21:26

Updates due to knip.

fbab6bc

Prettier.

b9f73fb

swensel commented Mar 26, 2026

View reviewed changes

Comment thread package.json

swensel commented Mar 26, 2026

View reviewed changes

Comment thread apps/main/package.json

swensel commented Mar 26, 2026

View reviewed changes

Comment thread packages/ui-common/__tests__/components/MultiAgentAccelerator/Sidebar.test.tsx Outdated

swensel commented Mar 26, 2026

View reviewed changes

swensel requested a review from Copilot March 26, 2026 19:27

Copilot started reviewing on behalf of swensel March 26, 2026 19:27 View session

swensel marked this pull request as ready for review March 26, 2026 19:32

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Update Sidebar.test.tsx.

4f6045b

swensel requested a review from dsargent March 26, 2026 19:54

dsargent requested changes Mar 26, 2026

View reviewed changes

swensel added 3 commits March 26, 2026 17:50

Remove test comment.

aa5d9d9

Update test comment.

a56d4e1

AgentFlow.test.tsx updates.

9b924a8

swensel changed the title ~~'Grab bag' PR to follow up on lower priority items from previous PRs. #235~~ Fix yarn install warnings, update unit tests, and update libraries. #235 Mar 27, 2026

swensel added 2 commits March 27, 2026 15:45

Move next out of root package.json. Move react-dom to dev deps in ui-…

ea7ee77

…common. Put eslint in dev deps in apps/main.

Remove some peer deps from ui-common package.json. Remove emotion pee…

df7afdb

…r deeps from apps/main package.json, and upgrade mui/system there.

swensel commented Mar 27, 2026

View reviewed changes

@emotion/react and @emotion/styled are optional peer deps @mui/x-tree…

a0ecbf3

…-view` but they do need to be included unless `styled-components` is included instead.

swensel commented Mar 28, 2026

View reviewed changes

swensel added 2 commits March 27, 2026 20:10

Remove next as a peer dep.

4db1119

Merge branch 'main' into grab-bag-PR

01b657e

swensel commented Mar 28, 2026

View reviewed changes

Adding next back to peer deps, knip says it's needed.

e27491e

swensel requested a review from dsargent March 28, 2026 03:30

swensel commented Mar 28, 2026

View reviewed changes

dsargent reviewed Mar 30, 2026

View reviewed changes

swensel added 3 commits March 31, 2026 23:18

Update ui-common typescript peer dep to 5.9.3

dcd686f

More knip workspace updates.

1858b9a

Add package.tgz to .gitignore.

6c5469b

swensel commented Apr 2, 2026

View reviewed changes

Comment thread .gitignore

# Build stuff

**/dist/

package.tgz

Copy link
Copy Markdown

Collaborator Author

swensel Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this to .gitignore.

swensel commented Apr 2, 2026

View reviewed changes

swensel requested a review from dsargent April 2, 2026 22:05

Merge branch 'main' into grab-bag-PR

baf2325

dsargent approved these changes Apr 3, 2026

View reviewed changes

swensel merged commit 0539356 into main Apr 3, 2026
8 checks passed

swensel deleted the grab-bag-PR branch April 3, 2026 20:32


		const TEST_AGENT_MUSIC_NERD_PRO = "Music Nerd Pro"

		const mockPlasmaEdgeTestId = "mock-plasma-edge"

Conversation

swensel commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swensel Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swensel Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

swensel Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swensel Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swensel Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsargent Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

swensel commented Mar 26, 2026 •

edited

Loading

swensel Mar 27, 2026 •

edited

Loading

swensel Mar 27, 2026 •

edited

Loading

swensel Mar 26, 2026 •

edited

Loading

swensel Apr 1, 2026 •

edited

Loading

swensel Mar 27, 2026 •

edited

Loading

dsargent Mar 26, 2026 •

edited

Loading