Skip to content

feat(catalog): surface incomplete channel metadata - #163

Merged
vincentkoc merged 7 commits into
openclaw:mainfrom
htydev:fix/catalog-integrity-diagnostics
Aug 3, 2026
Merged

feat(catalog): surface incomplete channel metadata#163
vincentkoc merged 7 commits into
openclaw:mainfrom
htydev:fix/catalog-integrity-diagnostics

Conversation

@htydev

@htydev htydev commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • surface orphaned message-to-channel catalog state in diagnostics
  • preserve missing metadata in human output and warn on unsafe empty raw-SQL identity results
  • add regression and 465,820-row calibration coverage

Verification

  • make check
  • make test-large

No archive repair, migration, restart, or live API operation is included.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Aug 3, 2026
@clawsweeper

clawsweeper Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 3, 2026, 2:01 AM ET / 06:01 UTC.

ClawSweeper review

What this changes

The PR adds read-only catalog-completeness diagnostics for messages missing channel metadata, preserves missing metadata in human output, warns on potentially misleading empty SQL results, and adds regression plus large-fixture coverage.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

This PR remains necessary: current main does not yet expose catalog completeness or warn that a zero-row SQL result may hide messages with missing channel metadata. The patch is focused, preserves read-only behavior, has no supported correctness finding, and now has maintainer-supplied real-archive validation on the current head; the remaining action is ordinary maintainer merge review.

Priority: P2
Reviewed head: 87272a38e68cc681c98921baf240da3bd5fbda87

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, well-covered patch with strong current-head operational proof is ready for ordinary maintainer merge review.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (logs): A repository member supplied redacted after-fix evidence from a real existing archive, including diagnostics and zero-row SQL behavior, alongside the 465,820-message calibration result; no sensitive archive details were disclosed.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (logs): A repository member supplied redacted after-fix evidence from a real existing archive, including diagnostics and zero-row SQL behavior, alongside the 465,820-message calibration result; no sensitive archive details were disclosed.
Evidence reviewed 6 items Current main does not contain the requested behavior: Current main is at 109e978, while the catalog completeness types, diagnostics fields, and SQL warning are introduced only on this PR head.
Focused read-only implementation: The diagnostics path opens the archive read-only, reports complete/incomplete/undetermined catalog state, and derives identity-query safety without changing archive data.
Empty-result warning is bounded: The SQL path only probes when a query returned zero rows; the probe has a two-second timeout and returns an undetermined note rather than failing the completed query.
Findings None None.
Security None None.

How this fits together

Discrawl stores Discord messages and channel records in a local SQLite archive. Its diagnostics command and read-only SQL interface help archive operators determine whether metadata completeness makes identity-filtered query results trustworthy.

flowchart LR
  Archive[Local Discord archive] --> Catalog[Messages and channel metadata]
  Catalog --> Probe[Catalog completeness probe]
  Probe --> Diagnostics[Diagnostics report]
  Probe --> SQL[Empty SQL result warning]
  Diagnostics --> Operator[Archive operator]
  SQL --> Operator
Loading

Before merge

  • Complete next step (P2) - No mechanical repair is warranted: the patch has no supported defect, and a human should make the normal merge decision for this new operator-facing capability.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch size 12 files affected; +546, -5 The change is broad enough to span CLI, store queries, generated SQL, docs, and tests, but remains centered on one catalog-integrity capability.
Production versus test growth production/docs/tooling +245, tests +301 Most added lines are regression and calibration coverage, matching the new diagnostic behavior's archive-scale claims.

Technical review

Best possible solution:

Land the focused diagnostics and warning path after normal maintainer review, keeping the archive strictly read-only and retaining the documented left-join guidance for queries that require channel metadata.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR adds a diagnostic capability rather than repairing a single reported current-main failure. The changed behavior has high-confidence real-archive validation and focused regression coverage.

Is this the best way to solve the issue?

Yes. A read-only completeness probe, clear diagnostics state, and a warning only for empty query results are a narrow maintainable way to surface the metadata limitation without attempting archive repair or a migration.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 109e97872147.

Labels

Label justifications:

  • P2: This is a bounded operator-facing archive diagnostics improvement with no evidence of an active data-loss, delivery, security, or availability incident.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): A repository member supplied redacted after-fix evidence from a real existing archive, including diagnostics and zero-row SQL behavior, alongside the 465,820-message calibration result; no sensitive archive details were disclosed.
  • proof: sufficient: Contributor real behavior proof is sufficient. A repository member supplied redacted after-fix evidence from a real existing archive, including diagnostics and zero-row SQL behavior, alongside the 465,820-message calibration result; no sensitive archive details were disclosed.

Evidence

What I checked:

  • Current main does not contain the requested behavior: Current main is at 109e978, while the catalog completeness types, diagnostics fields, and SQL warning are introduced only on this PR head. (internal/cli/diagnostics.go:17, 87272a38e68c)
  • Focused read-only implementation: The diagnostics path opens the archive read-only, reports complete/incomplete/undetermined catalog state, and derives identity-query safety without changing archive data. (internal/cli/diagnostics.go:185, 87272a38e68c)
  • Empty-result warning is bounded: The SQL path only probes when a query returned zero rows; the probe has a two-second timeout and returns an undetermined note rather than failing the completed query. (internal/store/query.go:85, 87272a38e68c)
  • Regression and scale coverage: Tests cover incomplete and complete diagnostics, zero-row warning behavior, missing-name human output, cancellation, timestamp ordering, and an opt-in 465,820-message complete-catalog calibration fixture. (internal/cli/catalog_integrity_test.go:15, 87272a38e68c)
  • Real behavior proof and maintainer validation: A repository member reported successful redacted read-only verification against an existing archive, a 465,820-message calibration run, diagnostics output, zero-row SQL behavior, and local quality gates on August 3, 2026. (87272a38e68c)
  • Feature-history routing: The existing CLI SQL isolation and output-helper boundaries trace to Peter Steinberger commits c4cb273 and 14b4b06; the new catalog behavior dates to this PR's commit series. (internal/cli/query_commands.go:276, c4cb273b203f)

Likely related people:

  • Peter Steinberger: Introduced the CLI foundation, output split, and the existing read-only SQL isolation boundary that this PR extends. (role: original CLI and SQL-safety contributor; confidence: high; commits: 26f9d0370520, 14b4b06a6527, c4cb273b203f; files: internal/cli/query_commands.go, internal/cli/output.go, internal/store/query.go)
  • Vincent Koc: Provided the current-head real-archive validation for this PR and has prior merged CLI work in the repository history. (role: recent area validator and adjacent CLI contributor; confidence: medium; commits: c4be70e52191; files: internal/cli/diagnostics.go, internal/cli/query_commands.go)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)
  • reviewed 2026-08-03T03:28:15.157Z sha 87272a3 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-03T04:11:14.404Z sha 87272a3 :: needs maintainer review before merge. :: none

@vincentkoc

Copy link
Copy Markdown
Member

Maintainer validation completed on August 3, 2026.

  • Local gates passed: module verification/tidy, formatting, golangci-lint, vet, staticcheck, gosec, govulncheck, 85.1% filtered coverage, race tests, and CLI smoke.
  • The release snapshot passed with GoReleaser v2.17.1 after supplying the tool from a temporary local tool directory.
  • make test-large passed with 465,820 messages; fixture creation took 22.1s and the healthy-catalog probe took 113.5ms.
  • A read-only run against an existing local archive reported catalog.state=complete, zero orphaned messages/channel IDs, and safe_for_identity_queries=true.
  • Direct read-only probes on that archive took about 20ms for the existence check and 30ms for the full catalog summary.
  • A zero-row identity join preserved JSON output and emitted no warning for the complete catalog.
  • Focused incomplete, complete, and undetermined catalog diagnostics/SQL tests all passed.

The evidence above is redacted: no archive path, guild/channel identity, message content, or credentials are included.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Aug 3, 2026
@clawsweeper clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Aug 3, 2026
@vincentkoc

Copy link
Copy Markdown
Member

Maintainer fixes are now on the contributor branch:

  • preserve make test-large failures instead of accepting a pass event from a failed go test
  • treat channel-row presence as authoritative, so blank valid names remain neutral while stale FTS names cannot hide missing metadata
  • replace the overbroad safe_for_identity_queries claim with the narrower catalog.state: consistent|incomplete|undetermined contract
  • document that catalog consistency covers stored message-to-channel references, not source-archive completeness
  • add the Unreleased changelog entry

Validation on 2e7cb6a:

  • focused CLI/store tests pass
  • lint, vet, staticcheck, gosec, and govulncheck pass with zero reachable vulnerabilities
  • coverage: 85.1%
  • 465k-row catalog fixture passes
  • smoke tests and six-platform GoReleaser snapshot pass
  • the one unrelated race-suite timeout passes 5/5 when rerun exactly under -race
  • read-only validation against an existing local archive reports a consistent catalog without exposing archive contents

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

* origin/main:
  docs: rewrite README to house standard (openclaw#162)

# Conflicts:
#	CHANGELOG.md
@vincentkoc

Copy link
Copy Markdown
Member

The branch head is now b75d89b after a signed merge of current main; the only conflict resolution preserved both Unreleased changelog sections. Focused CLI/store tests and git diff --check pass on the merged head, and CI, Docker, CodeQL, and secret scanning are green.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@vincentkoc vincentkoc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Validated on the exact head. The catalog contract now reports stored referential integrity without claiming source completeness; stale FTS names cannot hide missing metadata; the large-test target preserves failures. Local gates, large-fixture proof, release snapshot, and all hosted CI/security checks pass.

@vincentkoc
vincentkoc merged commit 3cae635 into openclaw:main Aug 3, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants