feat(ask): persist per-post conversation history - #675
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
85a1109
into
docs/adr-0133-0137-decomposition-matrix
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| post.post_title, post.visibility_code, post.corporate_entity_id, | ||
| post.author_account_id, post.source_detail_state_code |
There was a problem hiding this comment.
🔴 Reopening a saved conversation crashes process-unit-scoped readers
The source and citation reauthorization queries select visibility_code and corporate_entity_id but not process_unit_id. For a private post, _can_see_post then reads post["process_unit_id"] whenever the reader is scoped to process units, raising KeyError on the row and returning 500. Any saved turn that cites or sources a private post breaks the transcript endpoint for such readers. The citation query at post_ask_history.py has the same omission.
Prompt for agents
Both queries in _visible_post_ids_batch (the source branch and the citation branch) omit post.process_unit_id from their select lists. The can_see_post callback passed in is _can_see_post from main.py, which for a private post evaluates post["process_unit_id"] whenever the account has a non-empty process_unit_ids set. Because the column is absent from the fetched asyncpg Record, this raises KeyError and the fetch_conversation endpoint returns 500. Add post.process_unit_id to the select list in both the source and citation query branches, matching how gather_chat_sources selects process_unit_id in backend/app/post_chat_ingestion.py.
Was this helpful? React with 👍 or 👎 to provide feedback.
| post.post_title, post.visibility_code, post.corporate_entity_id, | ||
| post.author_account_id, post.source_detail_state_code |
There was a problem hiding this comment.
🔴 Asking a question crashes process-unit-scoped readers citing private posts
The citation reauthorization query selects visibility_code and corporate_entity_id but not process_unit_id. Inside the persist_turn commit, _can_see_post reads post["process_unit_id"] for a private cited post when the reader is scoped to process units, raising KeyError on the row. The error escapes uncaught, so the chat POST returns 500 and no turn is saved.
| post.post_title, post.visibility_code, post.corporate_entity_id, | |
| post.author_account_id, post.source_detail_state_code | |
| post.post_title, post.visibility_code, post.corporate_entity_id, | |
| post.author_account_id, post.source_detail_state_code, post.process_unit_id |
Was this helpful? React with 👍 or 👎 to provide feedback.
| rows = await conn.fetch( | ||
| """ | ||
| select relation.cited_post_id::text as post_id, | ||
| post.post_title, post.visibility_code, post.corporate_entity_id, | ||
| post.author_account_id, post.source_detail_state_code | ||
| from post_ask_turn_citation relation | ||
| join source_post post on post.post_id = relation.cited_post_id | ||
| where relation.post_ask_session_id = $1 | ||
| and relation.turn_ordinal = $2 | ||
| for share of post | ||
| """, | ||
| conversation_id, | ||
| turn_ordinal, | ||
| ) | ||
| if len(rows) != cited_post_count or any(not can_see_post(row) for row in rows): | ||
| raise PostAskEvidenceChanged |
There was a problem hiding this comment.
📝 Info: Write-path citation recheck skips draft/deleted eligibility
_ensure_citations_visible applies only can_see_post, while _visible_post_ids_batch also applies SOURCE_POST_ELIGIBILITY_SQL. A cited post that is ABAC-visible but draft/deleted would pass the commit-time recheck yet be dropped on read. Citations come from gather_chat_sources, which already excludes ineligible posts, so the asymmetry is currently harmless but inconsistent.
Was this helpful? React with 👍 or 👎 to provide feedback.
| conversation_id = await _persist_post_ask_turn( | ||
| conn, | ||
| account, | ||
| post_id, | ||
| request.conversation_id, | ||
| question, | ||
| stored["answer_text"], | ||
| source_ids, | ||
| list(stored["cited_post_ids"]), | ||
| ) |
There was a problem hiding this comment.
📝 Info: Cache-hit asks with no conversation id spawn new conversations each time
Every request persists a turn, including the stored cache-hit path. With request.conversation_id null, persist_turn creates a fresh post_ask_session per call, so repeatedly re-asking the same seeded question (e.g. clicking a suggestion chip) accumulates many single-turn conversations in the history list.
Was this helpful? React with 👍 or 👎 to provide feedback.
| async with pool.acquire() as conn: | ||
| await persist_post_chat(conn, post_id, question, answer.answer_text, cited_ids) | ||
| conversation_id = await _persist_post_ask_turn( | ||
| conn, | ||
| account, | ||
| post_id, | ||
| request.conversation_id, | ||
| question, | ||
| answer.answer_text, | ||
| source_ids, | ||
| cited_ids, | ||
| ) |
There was a problem hiding this comment.
📝 Info: Cache write and history write are not atomic
In the live path persist_post_chat commits before _persist_post_ask_turn opens its own transaction. If the turn persist raises (e.g. 503 on changed evidence), the post_chat_result cache row is committed while the account-history turn is not, leaving the two stores diverged. Impact is low because the cache is additive to history.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Contract
Implements the focused ADR 0136 gap as superseding ADR 0228 without replaying closed PR #490. LineageWeave persists account-owned post Ask transcripts; it does not perform psychometric or statistical arithmetic.
Verification
uv run --extra dev --extra backend pytest -q tests/test_post_ask_history.py(7 passed)corepack pnpm run lintandcorepack pnpm run buildThe broader local App suite reached 82 passes and 13 existing 5-second timeouts without assertion failures; hosted exact-head checks remain authoritative.
Stacked on #671 so the product-gap matrix lands before this implementation.