Repository navigation
feat(sources): preserve citation position locators - #2296
Tech-Circle-48 wants to merge 9 commits into
Conversation
hireview001
left a comment
There was a problem hiding this comment.
- CI failed: Upgrade release gate (failure).
- CI failed: upgrade-acceptance / Verify the selected upgrade matrix (failure).
- CI failed: Recall effort (OceanBase) (failure).
- CI failed: macos (macos-15) (failure).
- CI failed: windows (failure).
- CI failed: macos (macos-15-intel) (failure).
- CI failed: linux (failure).
- CI failed: tests (3.13, unit) (failure).
- CI failed: tests (3.11, unit) (failure).
- CI failed: tests (3.12, unit) (failure).
- Additional CI issues: PR checks.
hidb4ai
left a comment
There was a problem hiding this comment.
The populated legacy-database upgrade is blocked before the locator-table migration can run. The existing unresolved Desktop and OpenDAL findings also remain applicable. Required quality, unit, and upgrade checks need to pass before merge.
hidb4ai
left a comment
There was a problem hiding this comment.
@Tech-Circle-48 Re-reviewed 2d9eb62d. All four findings remain reproducible; I have added current evidence in their existing threads, including the OpenDAL finding currently marked resolved. Please fix the populated-database upgrade, locator decoding, versioned worker registration, and restore coverage, then resolve the merge conflicts and rerun the failing quality/unit/upgrade checks. Approval remains blocked.
|
|
||
| from alembic import op | ||
|
|
||
| revision = "v1_4_1_005" |
There was a problem hiding this comment.
[P1] v1_4_1_005 已被 master 占用,需要改号
master 上的 #2192(feat(recall): add bounded atomic memory manifest,master tip 90355b8e)已经合入 revision = "v1_4_1_005" 的 versions/v1_4_1_005_atomic_memory_description.py,并且同样新增了 schemas/sqlite-v1_4_1_005.json、schemas/mysql-v1_4_1_005.json、mysql/versions/v1_4_1_005.py。
本 PR 用的是同一个修订号、同一组路径,内容却不同:master 的 schemas/sqlite-v1_4_1_005.json 用 statement 键描述 pc_atomic_memory_current.description,本 PR 的同名文件用 create 键描述 pc_artifact_source_locators。相对旧 base 91805efd 本 PR 是自洽的,但 mergeable 现在是 CONFLICTING:变基到当前 master 后,这四个新增文件会逐一冲突;即使人工两边都保留,alembic 侧仍会同时存在两个 revision = "v1_4_1_005"、且 down_revision 都指向 v1_4_1_004 的脚本。
影响:受管数据库的升级链无法得到唯一的线性历史,db-migrate plan/apply 会在重复修订号上失败或落到不确定布局;manifest.json 的 sqlite_fingerprints、domain_readiness、mysql_state_layouts 每个分区也只有一个 v1_4_1_005 槽位,两份不同的 DDL 无法共存。
建议:本 PR 改号为 v1_4_1_006——迁移脚本文件名、mysql/versions/、两个 schemas/* 文件、manifest.json 各分区、vector_profile.sqlite_layouts 一并改,down_revision 指向 v1_4_1_005。变基后还要补齐两处随 base 变化的产物:release/upgrade-support.json 的 persistence_sha256(我已用 scripts/upgrade_release_gate.py::managed_digest 复算过当前值,改号后会变),以及桌面端生成物与资格记录——master 的 #2192 已把契约摘要改成 fd8f5dcf...,当前的 8f04516f...(operations.json、ui/src/generated/operations.ts)和 sqlite-atomic-d9686bba-v1 这条 compatibility.json 记录都会失配。
There was a problem hiding this comment.
@Tech-Circle-48 Rechecked at d50ba73a against current master 90355b8e. This remains blocking. Both actual migration scripts declare revision = "v1_4_1_005" and down_revision = "v1_4_1_004", but master adds pc_atomic_memory_current.description while this PR creates pc_artifact_source_locators. They also reuse the same schema-resource paths with incompatible statement/create contents. Loading both scripts with Alembic reproduces a duplicate-revision warning and a multiple-heads error. The latest documentation update and DMG retry revert do not address this collision. Please rebase and append the locator migration under a new revision after master's registered v1_4_1_005, preserving the existing frozen resources/checksums and updating both source paths and vector layouts, then rerun populated upgrade acceptance.
There was a problem hiding this comment.
Rechecked at ba222e35. Renumbering the locator step to v1_4_1_006 fixes the old collision against base 90355b8e, but it still collides with current master 21ce519f: master now registers v1_4_1_006_source_write_plugins.py (revision = v1_4_1_006, down_revision = v1_4_1_005) and then v1_5_0_001_artifact_storage.py. This PR declares the same v1_4_1_006 for a different table and reuses its schema-resource paths. Loading the actual scripts reproduces the duplicate-revision warning and multiple heads. Please rebase and append the locator change to the current registered chain under a unique revision, preserving the existing Source-plugin/Artifact-storage history and checksums, then rerun populated upgrade acceptance against that combined chain.
| in serialized references, preserving existing records and canonical encodings. Exact Artifact Revision and Source | ||
| eligibility checks still apply. | ||
|
|
||
| Server database revision `v1_4_1_005` adds the optional `pc_artifact_source_locators` relation keyed by the existing |
There was a problem hiding this comment.
[P2] 新的受管修订没有登记到迁移手册
本行宣告 v1_4_1_005 是新的受管修订,但操作者用来规划升级的 docs/en/docs/operate/database-migrations.md 里没有这个修订:第 19–20 行仍写两条已登记发布源路径终止于 v1_4_1_004,第 97–103 行的修订表也只到 v1_4_1_004,pc_artifact_source_locators 全文未出现;docs/zh/docs/operate/database-migrations.md:17-18 同样过期。
证据:tests/builtin/persistence/test_full_server_migrations.py 已改为断言 plan.revisions == ("s1400", "s1401", "s1402", "v1_4_1_004", "v1_4_1_005"),即 db-migrate plan 的真实目标已经不是文档写的 v1_4_1_004。
对比:本 PR 已经同步了 docs/*/workflows/sources.md 与 docs/*/operate/troubleshoot.md(OBLoader 分层表补了新表),只漏了迁移手册这一份。
建议:变基后(若按上一条改号则为 v1_4_1_006)在修订表补一行、把当前可用性列表的终止修订改到新号,并补一段说明新增可选表、已有血缘行与历史负载不重写、向量不受影响。中英两份都要改。
|
|
||
| @model_validator(mode="after") | ||
| def validate_span(self) -> SourceLineRangeLocator: | ||
| if self.end_line < self.start_line or self.end_line - self.start_line + 1 > 200: |
There was a problem hiding this comment.
[💭] 两个新上限没有登记到 limits.py
end_line - self.start_line + 1 > 200 与 heading_path 的 max_length=1024 直接写死在这里;仓库里同类上限都在 src/powercontext/limits.py(MAX_SOURCE_ID_LENGTH、MAX_SOURCE_TYPE_LENGTH、MAX_SOURCE_OBSERVATION_BYTES,连同样叫 locator 的 MAX_EXTERNAL_SKILL_LOCATOR_LENGTH 也在),而 limits.py 本身是 scripts/upgrade_release_gate.py 的 MANAGED_INPUTS 之一。
这两个数字现在同时出现在运行期模型、openapi/powercontext.yaml 的 description 和中英两份 docs/*/workflows/sources.md 正文里,改一处不会有任何检查提示其余几处。
建议:在 limits.py 增加 MAX_SOURCE_LOCATOR_LINES 与 MAX_SOURCE_HEADING_PATH_LENGTH,模型引用常量,描述文本随之同步。
hidb4ai
left a comment
There was a problem hiding this comment.
The remaining blocker is the conflicting registered migration revision with current master, confirmed again in the existing thread. Please rebase, give the locator migration its own successor revision, and validate the combined upgrade chain before merge. The prior four runtime/compatibility findings were verified fixed; the latest changes only update migration documentation and revert the unrelated DMG retry.
hireview001
left a comment
There was a problem hiding this comment.
- CI failed: windows (failure).
- CI failed: macos (macos-15) (failure).
- CI failed: macos (macos-15-intel) (failure).
- CI failed: linux (failure).
- CI failed: Desktop validation (failure). Failed steps: linux / Run cargo test --locked --manifest-path src-tauri/Cargo.toml; macos (macos-15-intel) / Run cargo test --locked --manifest-path src-tauri/Cargo.toml; macos (macos-15) / Run cargo test --locked --manifest-path src-tauri/Cargo.toml.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "contractSha256": "fd8f5dcf8dfb1a02c186623c08c3c62217ecdbc21a80cd5d9c329f3497b85f7d", | |||
| "contractSha256": "98285bffc8c8c5a2cea7f52c2fe1661d5edad5fef0af6d661da6ee86b0960338", | |||
There was a problem hiding this comment.
[P1] Qualify a Desktop profile for the regenerated contract
The new contract digest matches none of the bundled compatibility profiles, so ordinary Desktop builds cannot enable save or search: connection qualification fails and business operations return CompatibilityUnverified. Add a genuinely qualified profile matching this contract before shipping the regenerated Desktop transport.
There was a problem hiding this comment.
Confirmed at ba222e35: the generated and actual OpenAPI contract digest is 98285bffc8c8c5a2cea7f52c2fe1661d5edad5fef0af6d661da6ee86b0960338, with zero matching records in bundled compatibility.json. Base 90355b8e has a matching profile for its fd8f5dcf... contract, so this is introduced by the regenerated PR contract. All four Desktop platform jobs fail bundled_qualification_allows_explicit_scope_browsing at the same missing-profile assertion. In ordinary builds, ConnectionManager::check cannot select qualification evidence, and business-operation snapshots are rejected with CompatibilityUnverified. Please qualify the resulting contract with the real Server/native harness and add its actual source/wheel evidence; do not rewrite a historical profile digest merely to make this check pass.
hidb4ai
left a comment
There was a problem hiding this comment.
@Tech-Circle-48 Re-reviewed ba222e35. The migration revision now conflicts with the subsequently merged Source-plugin history on current master, and the regenerated Desktop contract still has no matching qualified profile. I added exact-head evidence to the existing threads. Please rebase onto the current migration chain and qualify the resulting Desktop contract before merging.
Which issue or RFC does this PR close?
Closes #1869
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?
How was this change tested?
AI usage statement