sledtools/pika branch #51

pika-git-chat-2

Stabilize branch discussion artifacts

Target branch: master

Merge Commit: d622cec2725528e54bc3cf94adc6a11cd2820452

branch: merged tutorial: ready ci: success

Open CI Details

Continuous Integration

CI: success

Compact status on the review page, with full logs on the CI page.

Open CI Details

Latest run #63 success

2 passed

head 6f5c8f0f8fe7d8a865efba16582e13dbd35b6cd0 · queued 2026-03-25 17:33:40 · 2 lane(s)

queued 11s · ran 29s

check-notifications · success check-agent-contracts · success

Summary

This branch stabilizes branch discussion (chat) artifacts by threading an explicit artifact_id through every layer of the chat system — from the database queries, through the web handlers, and into the frontend JavaScript. Previously, chat sessions were implicitly bound to whichever artifact was marked is_current at query time, meaning that if a new tutorial version was generated mid-conversation, the chat context could silently shift. Now, the artifact ID is resolved once when the detail page renders, passed to the client as branchChatArtifactId, and sent back with every chat history fetch and message send request, ensuring that each discussion thread is pinned to the exact artifact version the user is viewing.

Tutorial Steps

Add `current_artifact_id` to the branch detail data model

Intent: Expose the current artifact's database ID in `BranchDetailRecord` so that downstream layers (web handler, template) can reference the specific artifact version the user should chat against.

Affected files: crates/pika-news/src/branch_store.rs

Evidence

@@ -69,6 +69,7 @@ pub struct BranchFeedItem {
 #[allow(dead_code)]
 pub struct BranchDetailRecord {
     pub branch_id: i64,
+    pub current_artifact_id: Option<i64>,

@@ -545,6 +546,7 @@ impl Store {
         self.with_connection(|conn| {
             conn.query_row(
                 "SELECT br.id,
+                        ba_current.id,

@@ -588,21 +590,22 @@ impl Store {
                 |row| {
                     Ok(BranchDetailRecord {
                         branch_id: row.get(0)?,
-                        repo: row.get(1)?,
+                        current_artifact_id: row.get(1)?,
+                        repo: row.get(2)?,

The BranchDetailRecord struct gains a new field current_artifact_id: Option<i64>. The SQL query in the detail-fetch function is updated to select ba_current.id as the second column (index 1), and every subsequent column index is shifted by one to accommodate the insertion.

This field is Option<i64> because a branch may not yet have a ready artifact (e.g., the tutorial is still generating). Downstream consumers can check for None to decide whether to enable or disable the chat UI.

Pin `get_branch_review_artifact_session_id` to a specific artifact

Intent: Replace the implicit 'find the current artifact' query with an explicit lookup by artifact ID, removing the race condition where a new artifact could become current between page load and chat interaction.

Affected files: crates/pika-news/src/storage.rs

Evidence

@@ -720,15 +720,14 @@ impl Store {
     pub fn get_branch_review_artifact_session_id(
         &self,
         branch_id: i64,
+        artifact_id: i64,
     ) -> anyhow::Result<Option<String>> {
         self.with_connection(|conn| {
             conn.query_row(
                 "SELECT claude_session_id
                  FROM branch_artifact_versions
-                 WHERE branch_id = ?1 AND is_current = 1 AND status = 'ready'
-                 ORDER BY version DESC
-                 LIMIT 1",
-                params![branch_id],
+                 WHERE branch_id = ?1 AND id = ?2 AND status = 'ready'",
+                params![branch_id, artifact_id],

The function get_branch_review_artifact_session_id now requires an artifact_id: i64 parameter in addition to branch_id. The SQL query changes from filtering on is_current = 1 with ORDER BY version DESC LIMIT 1 to a direct lookup on id = ?2. This is the core of the stabilization fix: the caller decides which artifact to use, rather than the database query picking whatever is current at execution time.

Refactor `get_or_create_branch_review_chat_session` to accept `artifact_id` directly

Intent: Eliminate the internal sub-query that resolved the current artifact, replacing it with a caller-supplied artifact ID. This removes a second source of implicit artifact resolution and keeps the chat session creation deterministic.

Affected files: crates/pika-news/src/storage.rs

Evidence

@@ -739,23 +738,11 @@ impl Store {
 
     pub fn get_or_create_branch_review_chat_session(
         &self,
-        branch_id: i64,
+        artifact_id: i64,
         npub: &str,
         claude_session_id: &str,
     ) -> anyhow::Result<(i64, Vec<ChatMessage>)> {
         self.with_connection(|conn| {
-            let artifact_id: i64 = conn
-                .query_row(
-                    "SELECT id
-                     FROM branch_artifact_versions
-                     WHERE branch_id = ?1 AND is_current = 1 AND status = 'ready'
-                     ORDER BY version DESC
-                     LIMIT 1",
-                    params![branch_id],
-                    |row| row.get(0),
-                )
-                .context("lookup branch review artifact for chat session")?;

The first parameter changes from branch_id to artifact_id. The 12-line internal sub-query that resolved branch_id → artifact_id via is_current = 1 is completely removed. The function now trusts the caller to supply the correct artifact ID, which was already resolved at page-render time and is round-tripped through the client.

Existing tests are updated to pass artifact_id instead of branch_id, using a latest_branch_artifact_id helper to look up the value explicitly.

Introduce `BranchChatArtifactQuery` and `BranchChatSendRequest` types in the web layer

Intent: Add dedicated deserialization types so the branch chat endpoints can receive the artifact ID from the client — as a query parameter for GET (history) and as a JSON body field for POST (send).

Affected files: crates/pika-news/src/web.rs

Evidence

@@ -3973,6 +3982,17 @@ struct ChatSendRequest {
     message: String,
 }
 
+#[derive(serde::Deserialize)]
+struct BranchChatArtifactQuery {
+    artifact_id: i64,
+}
+
+#[derive(serde::Deserialize)]
+struct BranchChatSendRequest {
+    artifact_id: i64,
+    message: String,
+}

Two new request types are introduced:

BranchChatArtifactQuery — used by the GET /branch/{id}/chat endpoint to extract artifact_id from the URL query string.
BranchChatSendRequest — used by the POST /branch/{id}/chat endpoint to extract both artifact_id and message from the JSON body.

The send handler switches from the generic ChatSendRequest (which only contained message) to BranchChatSendRequest, ensuring the artifact ID is always present.

Thread `artifact_id` through the branch chat HTTP handlers

Intent: Wire the new artifact ID parameter from the request types into the storage calls, completing the backend plumbing so that every chat operation targets a pinned artifact.

Affected files: crates/pika-news/src/web.rs

Evidence

@@ -3906,6 +3908,7 @@ async fn branch_chat_history_handler(
     State(state): State<Arc<AppState>>,
     Path(branch_id): Path<i64>,
+    Query(query): Query<BranchChatArtifactQuery>,

@@ -3916,7 +3919,7 @@ async fn branch_chat_history_handler(
-        move || store.get_branch_review_artifact_session_id(branch_id)
+        move || store.get_branch_review_artifact_session_id(branch_id, query.artifact_id)

@@ -4135,7 +4155,8 @@ async fn branch_chat_send_handler(
-        move || store.get_branch_review_artifact_session_id(branch_id)
+        let artifact_id = body.artifact_id;
+        move || store.get_branch_review_artifact_session_id(branch_id, artifact_id)

@@ -4166,7 +4187,8 @@ async fn branch_chat_send_handler(
-        move || store.get_or_create_branch_review_chat_session(branch_id, &npub, &base_session_id)
+        let artifact_id = body.artifact_id;
+        move || store.get_or_create_branch_review_chat_session(artifact_id, &npub, &base_session_id)

Both branch_chat_history_handler and branch_chat_send_handler are updated:

History handler gains a Query(query): Query<BranchChatArtifactQuery> extractor. The query.artifact_id is forwarded to both get_branch_review_artifact_session_id and get_or_create_branch_review_chat_session.
Send handler switches its body type to Json<BranchChatSendRequest>. The body.artifact_id is captured into a local variable before the move closures (necessary because body is partially consumed for message), then passed into the same two storage functions.

In both cases, branch_id is still used for the session-ID lookup (as a safety filter), while artifact_id selects the exact artifact version.

Pass `branch_chat_artifact_id` to the detail page template

Intent: Supply the resolved artifact ID to the HTML template so it can be embedded as a JavaScript constant, making it available to the client-side chat logic.

Affected files: crates/pika-news/src/web.rs, crates/pika-news/templates/detail.html

Evidence

@@ -302,6 +302,7 @@ struct DetailTemplate {
     branch_id: i64,
+    branch_chat_artifact_id: Option<i64>,

@@ -2195,6 +2196,7 @@ fn render_detail_template_with_notices(
         branch_id: record.branch_id,
+        branch_chat_artifact_id: record.current_artifact_id,

@@ -587,6 +587,7 @@
         const branchId = {{ branch_id }};
+        const branchChatArtifactId = {% match branch_chat_artifact_id %}{% when Some with (artifact_id) %}{{ artifact_id }}{% when None %}null{% endmatch %};

The DetailTemplate struct gains a branch_chat_artifact_id: Option<i64> field, populated from record.current_artifact_id. In the Askama template, this is rendered as a JavaScript constant using a match expression: Some(id) emits the numeric ID, None emits null. This value is then available to the frontend chat functions.

Update frontend JavaScript to use the pinned artifact ID

Intent: Modify the client-side chat code to include the artifact ID in both the fetch (GET) and send (POST) requests, and to guard against missing artifact IDs with clear user-facing messages.

Affected files: crates/pika-news/templates/detail.html

Evidence

@@ -644,7 +645,7 @@
-          if (!branchChatReady || !discussionMessages) {
+          if (!branchChatReady || branchChatArtifactId === null || !discussionMessages) {

@@ -660,7 +661,7 @@
-            const res = await fetch(`/news/branch/${branchId}/chat`, {
+            const res = await fetch(`/news/branch/${branchId}/chat?artifact_id=${branchChatArtifactId}`, {

@@ -717,6 +718,14 @@
+          if (branchChatArtifactId === null) {
+            setDiscussionLocked(
+              'Discussion opens after tutorial generation.',
+              'This branch does not have a stable tutorial artifact id yet, so there is no discussion thread to send to.'
+            );
+            return;
+          }

@@ -732,7 +741,7 @@
-              body: JSON.stringify({ message })
+              body: JSON.stringify({ artifact_id: branchChatArtifactId, message })

Four changes in the template JavaScript:

Load guard — loadBranchDiscussion() now also checks branchChatArtifactId === null before attempting to fetch history. If null, the discussion panel shows a locked state.
History fetch — The GET URL changes from /news/branch/${branchId}/chat to /news/branch/${branchId}/chat?artifact_id=${branchChatArtifactId}.
Send guard — The send function adds an early return with a locked message if branchChatArtifactId is null, preventing sends when no artifact is available.
Send body — The POST body changes from { message } to { artifact_id: branchChatArtifactId, message }.

Add integration test for artifact-pinned chat history

Intent: Verify that requesting chat history with a specific artifact ID returns only the messages from that artifact's chat session, not the current/latest one.

Affected files: crates/pika-news/src/web.rs

Evidence

@@ -8288,6 +8310,109 @@ paths = ["README.md", "feature.txt", "ci/forge-lanes.toml"]
+    #[tokio::test]
+    async fn branch_chat_history_uses_requested_artifact_not_current_branch_state() {

@@ +8350,0 +8350,20 @@
+        let (first_session_id, _) = store
+            .get_or_create_branch_review_chat_session(
+                first_artifact_id,
+                TRUSTED_NPUB,
+                "branch-chat-session-1",
+            )
+            .expect("first chat session");
+        store
+            .append_branch_review_chat_message(first_session_id, "assistant", "artifact one")

@@ +8400,0 +8400,15 @@
+        let response = super::branch_chat_history_handler(
+            State(state),
+            Path(branch.branch_id),
+            Query(super::BranchChatArtifactQuery {
+                artifact_id: first_artifact_id,
+            }),
+            headers,
+        )
+        .await
+        .into_response();
+        assert_eq!(response.status(), StatusCode::OK);

@@ +8415,0 +8415,3 @@
+        assert!(text.contains("artifact one"));
+        assert!(!text.contains("artifact two"));

The test branch_chat_history_uses_requested_artifact_not_current_branch_state is the key correctness proof:

Creates a branch and generates two successive artifacts, each with their own chat session and a distinct message ("artifact one" and "artifact two").
Calls branch_chat_history_handler with artifact_id set to the first artifact — which is no longer current.
Asserts the response body contains "artifact one" and does not contain "artifact two".

This directly validates the fix: even though the second artifact is now is_current = 1, requesting chat history for the first artifact returns only that artifact's conversation.

pika-git-chat-2

CI: success

Summary

Tutorial Steps

Add `current_artifact_id` to the branch detail data model

Pin `get_branch_review_artifact_session_id` to a specific artifact

Refactor `get_or_create_branch_review_chat_session` to accept `artifact_id` directly

Introduce `BranchChatArtifactQuery` and `BranchChatSendRequest` types in the web layer

Thread `artifact_id` through the branch chat HTTP handlers

Pass `branch_chat_artifact_id` to the detail page template

Update frontend JavaScript to use the pinned artifact ID

Add integration test for artifact-pinned chat history

Diff