Repository navigation
feat(desktop): visualize current-Scope inventory and usage - #2220
Conversation
|
Please fix the CI failures and resolve the merge conflicts with the latest |
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
- CI did not complete: linux-systemd-user (cancelled).
- CI did not complete: Native personal service (cancelled).
hireview001
left a comment
There was a problem hiding this comment.
- CI failed: Upgrade release gate (failure).
- CI failed: upgrade-acceptance / Prepare upgrade artifacts and matrix (failure).
- CI failed: Main (failure). Failed steps: upgrade-acceptance / Prepare upgrade artifacts and matrix / Run upgrade gate regressions; Upgrade release gate / Require the selected parallel acceptance workflow.
There was a problem hiding this comment.
Reviewed head 434859f8d75dbe172812b67b0acdf96396c2a581.
The upgrade CI remains a merge blocker: with PC_UPGRADE_SOURCE_SHA inherited, test_parallel_workers_reject_changed_preparation_inputs[checkout] does not raise the expected GateError. This was reproduced locally, and current master already contains the fixture environment-isolation fix. Please integrate it and rerun the upgrade preparation and release gate.
Two inline comments cover the inventory destination/scope alignment and a non-blocking pending-state message. No blocking data, authorization, or context-isolation defect was found in the supported statistics path.
| refreshing={refreshing} | ||
| onRefresh={() => void refresh()} | ||
| /> | ||
| <Statistics |
There was a problem hiding this comment.
[P2] Connect the inventory view to the declared Observation destination
With a qualified connection, an exact active Scope, and get_stats support, navigating to Observation → Current-Scope inventory & usage still shows the unavailable placeholder: Statistics is mounted only on Overview, and the Observation branch receives no inventory panel. Issue #2106 explicitly calls for consuming the shared Observation container. Please mount this view through panels.inventory and cover the navigation. If maintainers accept an intermediate Overview-only delivery, explicitly confirm that scope and retain the remaining 04-B integration as open work.
There was a problem hiding this comment.
补充定位与成本证据,支持本条成立:
App.tsx:44的type Page = "home" | "connections" | "memories" | "observation" | "settings";Statistics只在page === "home"的分支里挂载(App.tsx:312–326),page === "observation"仍然渲染Observation(App.tsx:356)的三 tab 占位。- 缺口纯在前端路由:本 PR 已把
get_stats贯通transport/operations.json、build.rs显式能力表、capabilities/main.json、lib.rs注册与generate.mjs的wanted,sqlite-stats-bad4c0c2-v1(11 ops)也已进connections/compatibility.json。命令本身对可信主窗口开放,不受页面限制。 - 文档口径是"can mount"(
docs/en/development/desktop-statistics.md:15),而desktop/README.md声明 Observation 含 Current-Scope inventory & usage tab——现状是组件已可用、声明入口仍是占位。
即修复等价于把同一组件挂到 observation 的对应 tab,不需要新的原生面或契约变更。
There was a problem hiding this comment.
上一条 P2 在 head 89e15533 已闭合:App.tsx:358-375 的 page === "observation" 分支现在通过 panels.inventory 挂载 Statistics(key 为 statistics-${generation}),Observation.tsx:21 的三个 tab 里 inventory 拿到该面板,Overview 的挂载仍然保留。这与 desktop/README.md 声明的 "Current-Scope inventory & usage" 入口一致,不再需要"仅 Overview 交付"的例外确认。
本轮复核未发现新的阻断问题;Atomic Memory clean migration (OceanBase) 与 shard 1/3 的红是真实数据库 Lock wait timeout exceeded(job 124407532053/124414702642,tests/e2e/test_atomic_artifact_search_http.py::test_long_query_recovery_probe_preserves_lexical_coverage[oceanbase]),该用例不在本 PR 变更面内,属环境抖动,需重跑。
仍成立且不需要新线程的一条:Statistics.tsx:150 的 !enabled → t.unavailable 仍未区分 contextPending(P3,非阻断)。
| ) : !enabled ? ( | ||
| <p>{t.unavailable}</p> |
There was a problem hiding this comment.
[P3, non-blocking] Distinguish a pending context transition from missing qualification
contextPending participates in enabled, so rechecking a qualified connection or switching Scope temporarily renders the unqualified-connection message even though get_stats remains qualified. This recovers when the transition finishes, but gives the user the wrong reason for the pause. Please render a distinct localized pending state before this qualification branch, preserve the existing private-data clearing, and cover the displayed state during a transition.
hidb4ai
left a comment
There was a problem hiding this comment.
Rechecked head 434859f8d75dbe172812b67b0acdf96396c2a581 against current master aedf7beb1ecff2ea1fade6ccdd8e4bf6a298f799. The PR head has not changed. It is not ready to merge:
- The existing inventory navigation issue remains reproducible through the App: with an exact active Scope and qualified
get_stats, Overview offers Load statistics, but Observation → Current-Scope inventory & usage displays the unavailable placeholder. Connect the component throughpanels.inventory, or explicitly agree the intermediate delivery scope with maintainers. - A local merge-tree now reports eight conflicting files, including the transport adapter, generated wire/IPC types, compatibility records, and Server probe. Integrate current master and regenerate the affected contracts rather than dropping either side's operations or qualification records.
- The latest upgrade preparation still fails because
test_parallel_workers_reject_changed_preparation_inputs[checkout]does not raiseGateErrorwith inheritedPC_UPGRADE_SOURCE_SHA. Current master contains the fixture environment-isolation fix. Integrate it and rerun preparation and the release gate; the upgrade matrix remains unexecuted.
hidb4ai
left a comment
There was a problem hiding this comment.
The inventory navigation defect remains reproducible at 434859f8d75dbe172812b67b0acdf96396c2a581: a qualified exact Scope still gets the unavailable placeholder at Observation → Current-Scope inventory & usage. Please connect the Statistics component to that panel.
Requesting changes pending that fix and the outstanding merge conflicts and upgrade CI failure. The upgrade regression fails with inherited PC_UPGRADE_SOURCE_SHA and passes when it is cleared; current master contains the fixture isolation fix.
hidb4ai
left a comment
There was a problem hiding this comment.
Re-reviewed 1c7911c07e2464232166b794a4a1962413457aed. The Observation inventory route now reaches Statistics, and the previous merge conflicts and upgrade-gate failures are cleared. No blocking findings remain.
The existing pending-state wording note is non-blocking. Full signed/release platform qualification remains tracked separately.
|
Merge can be performed after resolving code conflicts. |
hidb4ai
left a comment
There was a problem hiding this comment.
Re-reviewed 9341edb1e6952d1c700fa6f65db68dc76d082366. No blocking correctness findings remain in the statistics path or its session-observation integration. The Observation inventory route, exact-Scope isolation, independent cancellation, and success/failure/cancellation observations remain intact. This PR reuses the existing Server statistics API and introduces no database schema changes.
Current master 838f726c149f520429ecf92399ea7e03b8fe5793 introduces a merge conflict confined to desktop/VALIDATION.md. Resolve it before merging, retaining both the new statistics qualification evidence and master's protocol-qualification heading/anchor. This approval covers the reviewed code; the branch is not yet directly mergeable.
The existing pending-state wording note remains non-blocking. Signed/release platform qualification remains separate.
|
Please resolve the remaining merge conflict with the latest |
hidb4ai
left a comment
There was a problem hiding this comment.
Re-reviewed f13d0656800d630126abb786a6e731a51dd4c215. No new statistics correctness defect was found, but this head is not ready to merge:
- Merging current master
90355b8e96d68e2932d7980b3e76957f35ea2b44conflicts in bothdesktop/VALIDATION.mdanddesktop/ui/src/generated/ipc.ts. Preserve both qualification records/anchors and regenerate IPC from the integrated native definitions, retaining the statistics commands and upstream types. - The quality check fails at
uv lock --project evaluation --locked. With CI's uv 0.10.12, the same failure reproduces on both this head and base91805efd28486676854f3bbd81a76f605752582d; the relevant dependency files are unchanged by this PR. This is a baseline lockfile problem, not a new statistics defect. Please coordinate the baseline fix and incorporate it before merging.
Please resolve the conflicts and complete the current-head checks, including the still-pending upgrade acceptance and release gate.
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
- CI failed: tests (3.12, unit) (failure).
- CI failed: tests (3.11, unit) (failure).
- CI failed: tests (3.14, unit) (failure).
- CI failed: tests (3.13, unit) (failure).
- CI failed: Main (failure). Failed steps: tests (3.13, unit) / Run unit tests; tests (3.14, unit) / Run unit tests; tests (3.11, unit) / Run unit tests.
|
@WuShang-d please fix the file confliction and ci failure. and then reply our review comments. |
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
| "method": "POST", | ||
| "path": "/v1/memory/search" | ||
| }, | ||
| "get_stats": { |
There was a problem hiding this comment.
[P1] get_stats 进入了需求清单,但没有任何同摘要资格记录能覆盖它 ⇒ 统计面板在真实连接上恒为不可用
operations.json:40 新增了 get_stats(POST /v1/stats),discovery.operationFeatures 也同步加了空特征列表,bundled contractSha256 为 37936d78…。问题在于:这份清单变了,资格记录没有跟着变。
资格选择的两个必要条件(desktop/src-tauri/src/connections/session.rs:351-360):
profile.compatibility.as_ref() == Some(&c.id)
&& contract["contractSha256"].as_str() == Some(&c.contract_sha256)其中 contract 来自 include_str!("../transport/operations.json"),即本仓 bundled 摘要,不是 Server 上报的摘要。
在 head 9e7d8d8d 上的实际匹配结果:
- 摘要等于
37936d78…的记录只有sqlite-source-artifact-87b6fae2-v1(feat(artifact): add managed plugin framework and some example #2246 合入),它的operations是 11 项,不含get_stats。 - 含
get_stats的四条sqlite-stats-*(1da99747/31ecdd61/bad4c0c2/27404931)摘要分别是5878999c…/3165f114…/c12901b8…,都不等于 bundled 摘要,find永远匹配不到。
于是 discovery.rs::assess(:159-176)对该 operation 走 known.is_some() 为 false 的分支,得到 Unverified(QualificationRequired);supported_operations()(:237)只收 Qualified,因此:
Statistics.tsx:57-61的enabled恒为 false ⇒ 面板恒显示unavailable("此连接尚无统计操作的兼容资格");- 即使绕过 UI 直接调 IPC,
session.rs::snapshot()(:408-416)也会因supported_operations不含get_stats返回CompatibilityUnverified。
为什么 CI 看不到这个问题:
desktop/tests/fixture_qualification.py:36-47直接从operations.json合成资格记录(contractSha256与operations同源,必然含get_stats),再由#[cfg(feature = "ci-fixtures")]注入;desktop/ui/tests/statistics.test.tsx:62手写supportedOperations: ["get_stats", "remember_memory"]。
两条路径都跳过了"按摘要选记录 → 按记录比对 operation"这一步,所以 411 行 tests/statistics.rs 与前端用例全绿并不覆盖产品资格。
建议:
- 为 bundled 摘要
37936d78…出一条包含get_stats的记录(复用sqlite-source-artifact-87b6fae2-v1的 Server 源与 wheel 身份,补统计端点验收证据),或在合并 master 后重新按当前契约出记录; - 增加一条不依赖
ci-fixtures的回归:断言"bundledcontractSha256存在一条记录,且其operations覆盖operations.json的全部 operation",这样下次契约摘要前移时会立刻打红,而不是让面板静默降级。
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
hidb4ai
left a comment
There was a problem hiding this comment.
Re-reviewed eb30a2defca43e159614d0820c170b71fe6404af. The previous merge conflicts, pending-context wording and bundled statistics qualification gap are cleared. The current product record covers all twelve bundled operations, and statistics rechecks Principal/deployment identity before and after the read. No new statistics correctness defect was found.
This head is still blocked by CI. The Python 3.11 unit job fails in test_concurrent_recovery_keeps_one_owner_and_one_audit: appending the access audit raises sqlite3.OperationalError: database is locked, then AccessUnavailableError. The test and related Python runtime paths are unchanged by this PR, so this is not evidence of a statistics regression. The aggregate CI result is red, while Desktop validation and upgrade acceptance were skipped and other matrix jobs were cancelled.
Please resolve or rerun the failing concurrency check and obtain the required current-head validation before merging. The local focused checks do not establish installed-platform or release acceptance.
|
Please resolve the code conflicts. |
hidb4ai
left a comment
There was a problem hiding this comment.
Re-reviewed daddf65887951c7f29a60cb8c43aef4ed9982715. No blocking findings remain. The Observation inventory and session panels coexist, and statistics operation names are localized in coverage and summaries. Exact-Scope selection, cancellation/identity protections and the matching product qualification remain intact. This reuses the existing Server API and introduces no database schema changes.
The previous owner-recovery failure is cleared in this head's Python 3.11 CI. Desktop platform checks and the upgrade release gate are still pending; this approval covers the reviewed code. Required checks must pass before merging. Installed/signed release qualification remains separate.
hidb4ai
left a comment
There was a problem hiding this comment.
Re-reviewed 31e5392a27944feaa08d89ddcca81b1a779d3518. No blocking findings remain. The new sqlite-stats-63e9665c-v1 record matches the bundled contract and covers all twelve Desktop operations, including statistics. Historical qualification identities remain unchanged, and both setup guides select the current profile. Exact-Scope isolation, independent cancellation, identity checks and the inventory/session integration remain intact. No Server API or database schema is added by this PR.
Current-head CI has no failed or pending checks. Installed-platform and signed-release qualification remain tracked separately.
Which issue or RFC does this PR close?
Implements current-Scope inventory and usage for #2106, following RFC #1455. Installed-platform and release acceptance remain tracked separately; this PR does not close the tracking issue.
Rationale for this change
Desktop needs explicit exact-Scope statistics through the existing Server API. Product qualification must authorize the operation for the bundled contract, and context transitions must hide private results and reject late responses.
What changes are included in this PR?
sqlite-stats-63e9665c-v1for all twelve Desktop operations, includingget_stats, at bundled digestd59da9040814d0f2b25d930ecb36b93d932ff8906440606d259d3b9456364d3d. All historical source/wheel/digest/operation records remain unchanged.ConnectionManager::newwithoutci-fixtures, then performs ordinary profile selection, connection checks, activation, exact-Scope selection and statistics reads. The harness verifies the actual wheel/digest/operation set and native product-record receipt; a stale fixture-only client is rejected.Are there any user-facing changes?
Yes. A deployment matching
sqlite-stats-63e9665c-v1can explicitly load statistics through the named Observation tab. Operators must match its recorded Server source and wheel. Other profiles do not gain statistics qualification automatically.Server APIs and persisted connection profile formats are unchanged. Snapshot inventory and recall estimates are not presented as billing savings or quality improvements.
How was this change tested?
63e9665cServer checkout and the ordinary product manager, not injected CI qualification. The Desktop report identifies the integrated working tree used for validation.63e9665c689fc7a37018dacb7f5236951a9a618d. Wheel SHA-256:0d4d9c5f52425a12debcedb18ff8596109e69a9fd1ca6025460c84d2b217db51. All 686 packaged runtime files match that source byte for byte. Artifact identity, commands and limits are recorded indesktop/VALIDATION.md#server-63e9665c-statistics-product-qualification.63e9665c, all 183 frontend tests, the complete native suite (two opt-in environment-dependent checks skipped), all-target Clippy, Rust formatting, OpenAPI generation and IPC drift checks passed with Node 24.14.1, pnpm 11.13.1 and Rust 1.95.0. Bilingual regressions cover inventory/session tab switching and statistics operation names.make docs-testpassed, including website tests, build/export and link validation.make checkpassed on integrated master63e9665c, including native-platform types, all hooks, lock checks, upgrade-policy checks and integration manifest checks.Host: macOS ARM64. Partial token reports are seeded Runtime-recorder inputs, not live model-provider calls. Installed Windows/Intel, signing/notarization, external production deployments and full upgrade/release acceptance require separate evidence. Current-head CI remains required.
AI usage statement
OpenAI Codex assisted with implementation, review, qualification and validation. Qoder CN performed independent read-only reviews of product qualification and the statistics/session UI integration.