Repository navigation
fix(dsh): center align dock panfix(dsh): center align dock panel above input bar and resolve sessionId from session propel above input bar and resolve session… - #766
Conversation
…Id from session prop
|
Hi maintainers 👋 This is my first contribution to memsearch. The PR (summarized in the description) fixes two issues when @zilliz/memsearch-dsh mounts in the DeepSeek Harness (DSH) Web GUI: dock-panel alignment and sessionId resolution from the session prop. One thing to flag: as a first-time contributor, the CI checks on this fork PR are pending maintainer approval and haven't executed yet. Could someone click “Approve and run workflows” so the lint/test checks run? Happy to address any review feedback. Thanks for your time! |
| { name: 'conversation.input.dock', id: 'skill-review', order: 15 }, | ||
| function (props) { | ||
| return React.createElement(SkillReviewPanel, { sessionId: props.sessionId }) | ||
| var sId = props.sessionId || (props.session && props.session.id) || (props.session && props.session.sessionId) |
There was a problem hiding this comment.
plugins/dsh/client.js line:677
Medium ---- The core behavior change of this PR — resolving sessionId from props.session when props.sessionId is absent — is not covered by any test. The existing harness in plugins/dsh/tests/client.test.js drives dockRenderer({ sessionId: 'session-1' }) (line 75), which only exercises the legacy sessionId branch. Please add a case such as dockRenderer({ session: { sessionId: 'session-2' } }) and assert that the panel (and the candidate fetch URL) use session-2; without it, a regression in the fallback chain would pass CI silently.
| /** The dock strip: candidate count + expandable review list. */ | ||
| function SkillReviewPanel(props) { | ||
| var sessionId = props.sessionId | ||
| var sessionId = props.sessionId || (props.session && props.session.id) || (props.session && props.session.sessionId) |
There was a problem hiding this comment.
plugins/dsh/client.js line:486
Low ---- This sessionId fallback is duplicated at the slot registration site (line 677), which already resolves the same expression and passes the result down as sessionId plus the raw session. Two copies of the same resolution logic that must be kept in sync, and the middle branch props.session && props.session.id is dead against the DSH contract: the dock owner passes a SessionSnapshot, which exposes sessionId, not id (see deepseek-ai/deepseek-harness packages/api/session-controller/src/client/contract/snapshot.ts). Consider resolving sessionId once in the registration renderer and passing only the resolved value (plus session when the panel still needs it), which removes the drift risk and the misleading .id branch.
| '--msr-warn:var(--dsw-alias-state-warn-primary,#f59e0b);' + | ||
| '--msr-error:var(--dsw-alias-state-error-primary,#ef4444);' + | ||
| 'box-sizing:border-box;' + | ||
| 'width:calc(100% - var(--dsh-composer-side-clearance, 16px) * 2);' + |
There was a problem hiding this comment.
plugins/dsh/client.js line:52
Low ---- The description says the width calculation matches DSH standard docks (TodoPanel/GoalBar), but it does not: those docks subtract the dock-inset as well — width: calc(100% - var(--dsh-composer-side-clearance) * 2 - var(--dsh-composer-dock-inset) * 4) and max-width: calc(var(--dsh-composer-card-max-width) - var(--dsh-composer-dock-inset) * 4) (see deepseek-ai/deepseek-harness TodoPanel.module.css/GoalBar.module.css). Here the width only removes the side clearances while padding adds back a single dock-inset inside a border-box width, so the panel content ends up one dock-inset wider per side than the sibling docks when they stack above the composer. If the goal is visual parity with the standard docks, subtract the four dock-insets from both width and max-width (and drop or keep the padding accordingly). If the wider alignment is intentional, the description is misleading.
| 'box-sizing:border-box;' + | ||
| 'width:calc(100% - var(--dsh-composer-side-clearance, 16px) * 2);' + | ||
| 'max-width:var(--dsh-composer-card-max-width, 780px);' + | ||
| 'margin:0 auto 6px;' + |
There was a problem hiding this comment.
plugins/dsh/client.js line:54
Low ---- This bottom margin interacts with the host's own spacing. The dock renders directly inside DSH's .composerStack, which already applies gap: var(--dsh-composer-stack-gap) (6px, ConversationRoot.module.css), so margin: 0 auto 6px leaves ~12px between the panel and the composer card, while the sibling docks (TodoPanel/GoalBar/QueueDock) rely on the stack gap alone (6px). If the extra 6px is intended breathing room that is fine, but it makes the vertical rhythm inconsistent with the docks this PR aligns against; otherwise consider dropping the bottom margin or using a negative margin like QueueDock does to normalize the spacing.
|
Thanks for the detailed review @yhmo! I have addressed all feedback in the latest commit:
|
|
Glad to hear that! Since all review comments are resolved and tests are green, could you please help approve and merge this PR when convenient? Thanks! （tips：I'm about to hardcore dive into coding）@yhmo |
|
Hi @zc277584121 👋 All review items on this PR have been verified and confirmed fixed by @yhmo (tests are 100% green). Could you please help merge this PR when you have a moment? Thank you! |
This branch has not been deployed
2 participants
Summary
This PR fixes two UI and integration issues when
@zilliz/memsearch-dshmounts in DeepSeek Harness (DSH) Web GUI:Alignment with Composer Card (
conversation.input.dock):.msr-rootlacked width and margin constraints, causing the collapsed capsule to be stuck to the far left screen edge and the expanded review panel to stretch inconsistently relative to the centeredcomposerCard.max-width: var(--dsh-composer-card-max-width, 780px),margin: 0 auto 6px, and side clearance calculations matching DSH standard docks (TodoPanel, GoalBar)..msr-capsule { align-self: flex-start; }so it aligns neatly above the composer card.Session Identification Resolution:
renderSlot("conversation.input.dock", zone)passes{ session, input }rather than a baresessionId.SkillReviewPaneland the slot registration to resolvesessionIdviaprops.sessionId || (props.session && props.session.id) || (props.session && props.session.sessionId).order: 15for consistent stacking between GoalBar (order: 10) and QueueDock (order: 20).