Skip to content

fix(web): delete sidebar sessions through the session action - #331

Merged
elkaix merged 1 commit into
mainfrom
fix/sidebar-session-delete
Sep 22, 2026
Merged

elkaix merged 1 commit into
mainfrom
fix/sidebar-session-delete

Conversation

@elkaix

@elkaix elkaix commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Related Issue

No tracked issue. Reported from the desktop app: sidebar delete showed "Pythinker daemon returned an error" and left the session in place.

Problem

Deleting a session from the sidebar failed in about 15ms. The session stayed in the list. The dialog had no error code and no daemon message.

What changed

The web client now posts the session delete action the daemon already implements. The shipped web bundle is rebuilt so the desktop app uses that call.

Permanent delete is unchanged. A busy-session guard and a design-system import are not in this change.

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked a related issue (external PRs: the issue must have a maintainer's /approve).
  • I have added tests that prove my feature works.
  • Ran gen-changesets skill, or this PR needs no changeset.
  • Ran gen-docs skill, or this PR needs no doc update.

Summary

  • What changed and why: sidebar delete now calls the existing session delete action instead of an HTTP method the daemon does not route.
  • User-visible behavior: deleting a session from the sidebar removes it instead of showing a daemon error.
  • Scope deliberately excluded: design-system import, and newer reference commits that do not fix this dialog.

Risk

  • Risk level and affected boundaries: low. One client method and the shipped web bundle. The daemon route is unchanged.
  • Failure, security, data, concurrency, dependency, and lifecycle considerations: a successful call still permanently deletes the session and its history. A missing session still returns the daemon not-found error, now with a code and a message.
  • New dependency or telemetry approval, if applicable: none.

Verification

  • Exact commands and outcomes: pnpm --filter @pymodel/pythinker-web exec vitest run test/daemon-client.test.ts — 17 passed. pnpm run build:web — copied the web bundle.
  • Tests added or updated: daemon-client.test.ts now expects POST /sessions/sess_1:delete with an empty JSON body.
  • Checks not run: full pnpm test, lint, typecheck, and Nix were not run. GitNexus detect_changes reported medium risk on DaemonPythinkerWebApi and the load process.

Rollback and review

  • Rollback path: revert this commit. The daemon route does not change.
  • Residual risk: a desktop build that still serves the old web bundle will keep the old call until it picks up this bundle.
  • Human review required: confirm the rebuilt bundle is the only generated change.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed session deletion from the sidebar.
    • Session deletion now works reliably with the updated server action.
  • Chores
    • Rebuilt web assets to ensure the latest application bundle loads correctly.

Sidebar delete called HTTP DELETE. The daemon only accepts POST /sessions/{id}:delete, so the desktop app showed a daemon error and left the session in place.
@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: PyModel/pythinker-code/.coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 51e68fd7-b51b-4357-bb09-3bdd452cd5c6

📥 Commits

Reviewing files that changed from the base of the PR and between 98235bb and 458119f.

⛔ Files ignored due to path filters (1)
  • apps/pythinker-code/dist-web/assets/index-DAFMRqR8.js is excluded by !**/assets/index-[0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-][0-9a-z_-].js
📒 Files selected for processing (19)
  • .changeset/sidebar-session-delete.md
  • apps/pythinker-code/dist-web/.web-bundle-manifest.json
  • apps/pythinker-code/dist-web/assets/CodeBlockNode-_wK8GSRB.js
  • apps/pythinker-code/dist-web/assets/DesignSystemView-BO8F15GQ.js
  • apps/pythinker-code/dist-web/assets/Tooltip-BV-JRC1N.js
  • apps/pythinker-code/dist-web/assets/index10-Dhnug51i.js
  • apps/pythinker-code/dist-web/assets/index11-BMhFgxFV.js
  • apps/pythinker-code/dist-web/assets/index3-BLmGaoDW.js
  • apps/pythinker-code/dist-web/assets/index3-U5vX94Wy.js
  • apps/pythinker-code/dist-web/assets/index4-BQHXvCib.js
  • apps/pythinker-code/dist-web/assets/index4-Wk5pGiJ7.js
  • apps/pythinker-code/dist-web/assets/index5-CCrZjPWg.js
  • apps/pythinker-code/dist-web/assets/index6-CGYWuLMy.js
  • apps/pythinker-code/dist-web/assets/index7-DOLmlvs-.js
  • apps/pythinker-code/dist-web/assets/index8-BKeFA_Sf.js
  • apps/pythinker-code/dist-web/assets/index9-BNCeHK1v.js
  • apps/pythinker-code/dist-web/index.html
  • apps/pythinker-web/src/api/daemon/client.ts
  • apps/pythinker-web/test/daemon-client.test.ts
💤 Files with no reviewable changes (2)
  • apps/pythinker-code/dist-web/assets/index4-BQHXvCib.js
  • apps/pythinker-code/dist-web/assets/index3-BLmGaoDW.js

Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.


📝 Walkthrough

Walkthrough

Changes

Session deletion update

Layer / File(s) Summary
Session deletion request contract
apps/pythinker-web/src/api/daemon/client.ts, apps/pythinker-web/test/daemon-client.test.ts, .changeset/sidebar-session-delete.md
deleteSession now sends POST /sessions/{id}:delete with an empty JSON body. The test validates the request and response. The changeset records the fix.
Generated web bundle refresh
apps/pythinker-code/dist-web/...
Generated asset references now use the rebuilt hashed bundles. CodeBlockNode and Tooltip entry modules were replaced with updated hashed files.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 45811

Sidebar session deletion now uses the daemon’s supported action endpoint, and the rebuilt web assets reference the updated modules. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 215 functions across 13 files. (3 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title uses the required fix prefix, stays within 72 characters, and clearly describes the sidebar session deletion fix in imperative form.
Description check ✅ Passed The description follows the template and documents the problem, implementation, testing, risk, verification, rollback, and scope. It states that no tracked issue exists and leaves the related-issue ch…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 215 functions across 13 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Sep 22, 2026

Copy link
Copy Markdown
pnpm dlx https://pkg.pr.new/@pymodel/pythinker-code@458119f
npx https://pkg.pr.new/@pymodel/pythinker-code@458119f

commit: 458119f

@elkaix
elkaix merged commit ea4bdd6 into main Sep 22, 2026
27 checks passed
@elkaix
elkaix deleted the fix/sidebar-session-delete branch September 22, 2026 16:25
elkaix added a commit that referenced this pull request Sep 25, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @pymodel/pythinker-code@2.2.1

### Patch Changes

- [#331](#331)
[`ea4bdd6`](ea4bdd6)
Thanks [@elkaix](https://github.com/elkaix)! - Fix deleting a session
from the sidebar.

- [#329](#329)
[`98235bb`](98235bb)
Thanks [@elkaix](https://github.com/elkaix)! - Tower mode: mission
titles must be printable ASCII, tower messages, findings, and reviews
record the sender's token usage, and task cards show the mission id.
## @pymodel/pythinker-desktop@1.3.1

### Patch Changes

- [#331](#331)
[`ea4bdd6`](ea4bdd6)
Thanks [@elkaix](https://github.com/elkaix)! - Fix deleting a session
from the sidebar.

- [#329](#329)
[`98235bb`](98235bb)
Thanks [@elkaix](https://github.com/elkaix)! - Tower mode: mission
titles must be printable ASCII, tower messages, findings, and reviews
record the sender's token usage, and task cards show the mission id.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Mohamed Elkholy <melkholy@techmatrix.com>
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.

1 participant