Skip to content

feat/fix: Amber feedback backlog — ZTD-1818/1820/1821/1822/1823/1824#11

Merged
yuzushi-dev merged 10 commits intomainfrom
fix/feedback-backlog
Apr 22, 2026
Merged

feat/fix: Amber feedback backlog — ZTD-1818/1820/1821/1822/1823/1824#11
yuzushi-dev merged 10 commits intomainfrom
fix/feedback-backlog

Conversation

@yuzushi-dev
Copy link
Copy Markdown
Owner

Summary

Implements 6 items from the Amber feedback backlog (ZTD-1817).

Ticket Type Change
ZTD-1824 Fix Pending feedback cards now show ThumbsDown (red) or ThumbsUp based on actual polarity — is_positive was missing from admin API response
ZTD-1818 Feat Tenant admins see only their own feedback; super admins get cross-tenant view with tenant badge on each card
ZTD-1823 Feat Users can re-submit and change feedback: same polarity → upsert, polarity flip → replace old PENDING, VERIFIED records never touched
ZTD-1821 Feat "View Document" button on document detail page renders the original file inline (PDF/HTML via iframe, Markdown via react-markdown, text via pre)
ZTD-1822 Feat History panel in client view (/amber/chat): slide-in drawer with paginated past conversations, active highlighting, delete, "New Chat"
ZTD-1820 Fix Cross-session memory bleed fixed: ConversationSummaries from past sessions no longer injected into new prompts; UserFacts (user profile) retained

Test plan

  • All 25 new unit tests pass (pytest tests/unit/test_feedback_*.py tests/unit/test_document_content_type.py tests/unit/test_chat_history_endpoint.py tests/unit/test_memory_session_isolation.py)
  • ZTD-1824: Open admin feedback page, verify positive/negative cards show correct icon and color
  • ZTD-1818: Log in as super admin, verify feedback from multiple tenants appears with tenant badge
  • ZTD-1823: Submit feedback, resubmit with same polarity (updates comment), flip polarity (replaces record)
  • ZTD-1821: Open a document detail page, click "View Document", verify PDF/MD/HTML renders inline
  • ZTD-1822: Open /amber/chat, click History icon, verify past conversations listed and clickable
  • ZTD-1820: Start a new chat after previous sessions, verify no "PAST CONVERSATIONS" section in prompt (check logs)

🤖 Generated with Claude Code

danieleveri-zextras and others added 10 commits April 17, 2026 10:15
These were never pytest test modules - they are smoke scripts with
asyncio entrypoints used to probe provider factory, NIM timeout, and
OpenRouter behaviour by hand. Having them under src/ meant they got
included in the Docker image and cluttered the production package.

Refs: docs/plans/2026-04-17-production-audit-fix-plan.md (P1-3)
…utdown

Neo4jClient and MilvusVectorStore both expose close(), not aclose()
(only the Redis async client has aclose). The wrong method name
raised AttributeError on every Platform.shutdown(), leaving SQLAlchemy
pool connections un-returned and triggering the follow-up
"non-checked-in connection ... will be terminated" errors from the GC.

Refs: docs/plans/2026-04-17-production-audit-fix-plan.md (P1-2)
RLS policies on tenant tables read current_setting(\"app.current_tenant\")
without the missing_ok flag, so any request that reached get_db_session
without a bound tenant raised "unrecognized configuration parameter"
42704 on the first query. The super-admin cross-tenant metrics path in
admin/maintenance was the loudest symptom, logging "Failed to resolve
document names" for every call.

Fix: always emit set_config(), using an empty string when no tenant is
bound. Behaviour for normal tenant-scoped requests is unchanged.

Refs: docs/plans/2026-04-17-production-audit-fix-plan.md (P1-1)
Two startup guardrails:

- CORS: a missing CORS_ORIGINS config used to silently fall back to
  "*". With DEBUG=false we now refuse to boot and raise RuntimeError,
  forcing the operator to set an explicit allow-list. DEBUG=true keeps
  the permissive default with a warning log.
- SECRET_KEY_OLD: log a warning when the rotation-fallback secret is
  one of the well-known non-entropic dev defaults (amber-dev-key-2024,
  default-insecure-key). Those values defeat the dual-key keyring and
  leave legacy hashes forgeable.

Refs: docs/plans/2026-04-17-production-audit-fix-plan.md (P1-4, P1-5)
…arity (ZTD-1824)

Pending feedback endpoint was not returning is_positive, causing the admin
UI to always display a ThumbsUp icon regardless of whether the feedback was
positive or negative. Both polarities were already stored correctly; the
field was simply missing from the API response.

- api/routes/admin/feedback.py: include is_positive in get_pending_feedback response
- api-admin.ts: add is_positive to FeedbackItem type
- FeedbackPage.tsx: render ThumbsDown (red) or ThumbsUp (primary) based on is_positive
…ew for super admins (ZTD-1818)

Admin sees only their tenant's pending feedback (already enforced by SQL WHERE).
Super admin gets cross-tenant visibility: no tenant filter in query, tenant_id
included in every response item and shown as a badge in the UI card.

- admin/feedback.py: add Request param; skip tenant WHERE clause for super_admin
- admin/feedback.py: include tenant_id in each pending-feedback response item
- api-admin.ts: add tenant_id to FeedbackItem type
- FeedbackPage.tsx: show tenant_id badge (amber) on cards when user is super admin
…r (ZTD-1823)

Users were permanently blocked from changing their feedback by a frontend
disabled state. Backend now supports smart upsert semantics:

- Same polarity re-submit (PENDING/NONE): updates comment/score in place
- Polarity flip (PENDING/NONE): deletes old record, creates new PENDING
- VERIFIED/REJECTED records: never touched; new PENDING record created

Frontend: removed permanent disabled state on thumbs buttons (only disabled
while request is in-flight), allowing users to change their feedback.

Feedback id is now generated explicitly on construction (not deferred to flush)
to ensure it is available before DB commit.
…D-1821)

The document detail page previously showed only RAG metadata (chunks, entities,
relationships). Users had no way to view the actual source document content.

The API endpoint GET /documents/{id}/file already existed. This change adds:
- DocumentViewer component: fetches the file with auth, renders inline
  - PDF / HTML → <iframe> with blob URL
  - Markdown → ReactMarkdown with GFM
  - Plain text → <pre> block
  - Unknown types → download fallback
- "View Document" button in the DocumentDetailPage header
- Download button always available inside the viewer dialog
…D-1822)

The client view (/amber/chat) had no way to access past conversations.
The backend GET /chat/history endpoint existed but was unwired in the UI.

- ChatHistoryPanel: slide-in panel from the left with paginated conversation
  list, active conversation highlight, delete with confirmation, and
  "New conversation" shortcut
- ClientLayout: adds History icon button in header that toggles the panel
- Backend already correct; tests confirm user+tenant scoping and response shape
… context (ZTD-1820)

Partners managing multiple setups experienced context bleed: ConversationSummaries
from session A (e.g., client A config) were injected into session B, causing
incorrect context and hallucinated answers about the wrong configuration.

Root cause: get_recent_summaries() was called on every generation request and its
results injected into the prompt under "PAST CONVERSATIONS". This is cross-session
by definition — the summaries span all past conversations for the user.

Fix: remove get_recent_summaries() injection from both the standard RAG path and
the streaming path. UserFacts (long-term user profile) are intentionally kept as
they represent persistent knowledge about who the user is, not what they discussed.
Comment thread src/api/main.py
"SECRET_KEY_OLD is set to a known dev default (%s). "
"Complete the rotation and unset SECRET_KEY_OLD, or replace "
"it with the actual previous secret being retired.",
settings.secret_key_old[:12] + "…",
@yuzushi-dev yuzushi-dev merged commit 29b4242 into main Apr 22, 2026
3 of 5 checks passed
yuzushi-dev added a commit that referenced this pull request Apr 22, 2026
- DocumentViewer: tighten HTML iframe sandbox from allow-same-origin to ''
  to prevent XSS via arbitrary HTML documents
- ChatHistoryPanel: remove unused bottomRef (dead code from incomplete
  IntersectionObserver implementation)
- admin/feedback.py: remove redundant query alias (query = base_query)
- feedback.py: set golden_status=PENDING on same-polarity upsert so
  re-submitted feedback is re-queued for admin review
- test_feedback_multiple_submissions: assert golden_status after upsert
- test_memory_session_isolation: add source-inspection regression guard
  that fails if get_recent_summaries re-appears in generation_service
yuzushi-dev added a commit that referenced this pull request Apr 22, 2026
- DocumentViewer: tighten HTML iframe sandbox from allow-same-origin to ''
  to prevent XSS via arbitrary HTML documents
- ChatHistoryPanel: remove unused bottomRef (dead code from incomplete
  IntersectionObserver implementation)
- admin/feedback.py: remove redundant query alias (query = base_query)
- feedback.py: set golden_status=PENDING on same-polarity upsert so
  re-submitted feedback is re-queued for admin review
- test_feedback_multiple_submissions: assert golden_status after upsert
- test_memory_session_isolation: add source-inspection regression guard
  that fails if get_recent_summaries re-appears in generation_service
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.

3 participants