-
Notifications
You must be signed in to change notification settings - Fork 0
.pr_agent_accepted_suggestions
| PR 30 (2026-06-06) |
[correctness] SetVisibility spec contradiction
SetVisibility spec contradiction
The design spec states `SetVisibility` should update visibility via `Upsert (no re-embed)`, but `Upsert` implies rewriting vectors and contradicts the plan, which correctly uses `SetPayload` to preserve vectors. If an implementer follows the spec literally, they may introduce unnecessary embedding work and vector churn and diverge from the planned tests/behavior.The spec describes SetVisibility as using Upsert (no re-embed), which is internally inconsistent (Upsert typically requires vectors) and contradicts the implementation plan that uses Qdrant SetPayload to flip visibility without re-embedding. This mismatch can mislead implementers and cause an incorrect/performance-regressive implementation.
The plan’s Task 7 includes a concrete SetVisibility implementation using s.client.SetPayload(...) and explicitly notes it preserves the vector. The spec should be updated to match that approach (or the plan should be changed, but the plan’s approach is the one that actually avoids re-embedding).
- docs/superpowers/specs/2026-06-06-per-actor-memory-isolation-design.md[167-176]
- docs/superpowers/plans/2026-06-06-per-actor-memory-isolation.md[869-886]
[maintainability] ADR newline escapes
ADR newline escapes
The newly added ADRs include literal `\n` sequences inside markdown list/paragraph text, so rendered ADRs will show “\n” and collapse intended multi-line bullets into one malformed line. This degrades readability and can break docs tooling expectations for list formatting.Four newly added ADR markdown files contain literal \\n escape sequences in the body text (bullets/paragraphs). In Markdown, this renders as the characters backslash+n rather than a newline, collapsing lists and making the ADRs hard to read.
These files are marked as rendered outputs (adr-render: ... do not edit manually). The correct fix is typically to update the bead/source or renderer so it emits real newlines, then re-render; as a stopgap, you can manually replace the \\n sequences with actual line breaks and proper list structure.
- docs/adr/engram-cgb-enforce-per-actor-authorization-store-layer-not-handlers.md[21-29]
- docs/adr/engram-hvg-use-stable-oidc-sub-as-authorization-key-new-owner-field.md[21-29]
- docs/adr/engram-kyz-sharing-grants-read-but-never-write-read-write-gate-asymmetr.md[21-29]
- docs/adr/engram-xa6-return-404-not-found-unauthorized-id-addressed-operations.md[21-29]