huskies: merge 729_story_store_story_name_as_a_crdt_field_separate_from_the_story_id
This commit is contained in:
@@ -38,7 +38,7 @@ pub use types::{
|
|||||||
CrdtEvent, NodePresenceCrdt, NodePresenceView, PipelineDoc, PipelineItemCrdt, PipelineItemView,
|
CrdtEvent, NodePresenceCrdt, NodePresenceView, PipelineDoc, PipelineItemCrdt, PipelineItemView,
|
||||||
subscribe,
|
subscribe,
|
||||||
};
|
};
|
||||||
pub use write::write_item;
|
pub use write::{migrate_names_from_slugs, name_from_story_id, write_item};
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
pub use state::init_for_test;
|
pub use state::init_for_test;
|
||||||
|
|||||||
@@ -10,6 +10,99 @@ use super::state::{apply_and_persist, emit_event, get_crdt, rebuild_index};
|
|||||||
use super::types::{CrdtEvent, PipelineDoc, PipelineItemCrdt};
|
use super::types::{CrdtEvent, PipelineDoc, PipelineItemCrdt};
|
||||||
use crate::slog;
|
use crate::slog;
|
||||||
|
|
||||||
|
// ── Name migration helpers ────────────────────────────────────────────
|
||||||
|
|
||||||
|
/// Derive a human-readable name from a story ID's slug component.
|
||||||
|
///
|
||||||
|
/// Strips the numeric prefix and item-type prefix (story/bug/spike/refactor),
|
||||||
|
/// replaces underscores with spaces, and capitalises the first letter.
|
||||||
|
///
|
||||||
|
/// Examples:
|
||||||
|
/// - `"729_story_store_story_name"` → `"Store story name"`
|
||||||
|
/// - `"4_bug_login_crash"` → `"Login crash"`
|
||||||
|
/// - `"10_spike_arch_review"` → `"Arch review"`
|
||||||
|
pub fn name_from_story_id(story_id: &str) -> String {
|
||||||
|
// Strip the leading digits then the first underscore: "729_story_..." → "story_..."
|
||||||
|
let after_num = story_id.trim_start_matches(|c: char| c.is_ascii_digit());
|
||||||
|
let after_num = after_num.strip_prefix('_').unwrap_or(after_num);
|
||||||
|
|
||||||
|
// Strip the item-type prefix.
|
||||||
|
let slug = after_num
|
||||||
|
.strip_prefix("story_")
|
||||||
|
.or_else(|| after_num.strip_prefix("bug_"))
|
||||||
|
.or_else(|| after_num.strip_prefix("spike_"))
|
||||||
|
.or_else(|| after_num.strip_prefix("refactor_"))
|
||||||
|
.unwrap_or(after_num);
|
||||||
|
|
||||||
|
// Replace underscores with spaces.
|
||||||
|
let spaced = slug.replace('_', " ");
|
||||||
|
|
||||||
|
// Capitalise the first character.
|
||||||
|
let mut chars = spaced.chars();
|
||||||
|
match chars.next() {
|
||||||
|
None => String::new(),
|
||||||
|
Some(first) => {
|
||||||
|
let mut name = first.to_uppercase().to_string();
|
||||||
|
name.push_str(chars.as_str());
|
||||||
|
name
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Backfill the `name` CRDT field for pipeline items that have an empty name.
|
||||||
|
///
|
||||||
|
/// Iterates over all items in the in-memory CRDT. For each item whose `name`
|
||||||
|
/// register is empty, derives a human-readable name from the story ID slug
|
||||||
|
/// (see [`name_from_story_id`]) and writes it via a signed CRDT op.
|
||||||
|
///
|
||||||
|
/// This is a one-time startup migration: items created before the `name` field
|
||||||
|
/// was consistently populated will gain a name on the next server start.
|
||||||
|
/// Items that already have a non-empty name are left untouched.
|
||||||
|
pub fn migrate_names_from_slugs() {
|
||||||
|
let Some(state_mutex) = get_crdt() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
// First pass: collect (index, derived_name) pairs for items missing a name.
|
||||||
|
let migrations: Vec<(usize, String)> = {
|
||||||
|
let Ok(state) = state_mutex.lock() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
state
|
||||||
|
.index
|
||||||
|
.iter()
|
||||||
|
.filter_map(|(story_id, &idx)| {
|
||||||
|
let item = &state.crdt.doc.items[idx];
|
||||||
|
// Skip items that already have a name.
|
||||||
|
let already_named =
|
||||||
|
matches!(item.name.view(), JsonValue::String(ref s) if !s.is_empty());
|
||||||
|
if already_named {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let name = name_from_story_id(story_id);
|
||||||
|
if name.is_empty() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
Some((idx, name))
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
};
|
||||||
|
|
||||||
|
if migrations.is_empty() {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Second pass: apply all name writes while holding the lock.
|
||||||
|
let Ok(mut state) = state_mutex.lock() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let count = migrations.len();
|
||||||
|
for (idx, name) in migrations {
|
||||||
|
apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].name.set(name.clone()));
|
||||||
|
}
|
||||||
|
slog!("[crdt] Migrated names for {count} items from story ID slugs");
|
||||||
|
}
|
||||||
|
|
||||||
/// Write a pipeline item state through CRDT operations.
|
/// Write a pipeline item state through CRDT operations.
|
||||||
///
|
///
|
||||||
/// If the item exists, updates its registers. If not, inserts a new item
|
/// If the item exists, updates its registers. If not, inserts a new item
|
||||||
@@ -157,6 +250,118 @@ mod tests {
|
|||||||
use super::super::state::init_for_test;
|
use super::super::state::init_for_test;
|
||||||
use super::super::state::rebuild_index;
|
use super::super::state::rebuild_index;
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
// ── name_from_story_id tests ─────────────────────────────────────────────
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn name_from_story_id_story_type() {
|
||||||
|
assert_eq!(
|
||||||
|
name_from_story_id("729_story_store_story_name_as_a_crdt_field"),
|
||||||
|
"Store story name as a crdt field"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn name_from_story_id_bug_type() {
|
||||||
|
assert_eq!(name_from_story_id("4_bug_login_crash"), "Login crash");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn name_from_story_id_spike_type() {
|
||||||
|
assert_eq!(name_from_story_id("10_spike_arch_review"), "Arch review");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn name_from_story_id_refactor_type() {
|
||||||
|
assert_eq!(
|
||||||
|
name_from_story_id("99_refactor_decompose_server"),
|
||||||
|
"Decompose server"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn name_from_story_id_single_word() {
|
||||||
|
assert_eq!(name_from_story_id("1_story_auth"), "Auth");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn name_from_story_id_unknown_type_fallback() {
|
||||||
|
// Unknown type prefix is left as-is after stripping the number.
|
||||||
|
assert_eq!(name_from_story_id("5_unknown_foo_bar"), "Unknown foo bar");
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── migrate_names_from_slugs tests ───────────────────────────────────────
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn migrate_names_from_slugs_fills_empty_names() {
|
||||||
|
init_for_test();
|
||||||
|
|
||||||
|
// Write an item without a name.
|
||||||
|
write_item(
|
||||||
|
"42_story_my_feature",
|
||||||
|
"1_backlog",
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Before migration the name should be empty.
|
||||||
|
let before = read_item("42_story_my_feature").unwrap();
|
||||||
|
assert!(
|
||||||
|
before.name.as_deref().unwrap_or("").is_empty(),
|
||||||
|
"name should be empty before migration"
|
||||||
|
);
|
||||||
|
|
||||||
|
migrate_names_from_slugs();
|
||||||
|
|
||||||
|
// After migration the name should be derived from the slug.
|
||||||
|
let after = read_item("42_story_my_feature").unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
after.name.as_deref(),
|
||||||
|
Some("My feature"),
|
||||||
|
"name should be derived from slug after migration"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn migrate_names_from_slugs_leaves_existing_names_unchanged() {
|
||||||
|
init_for_test();
|
||||||
|
|
||||||
|
write_item(
|
||||||
|
"43_story_named_item",
|
||||||
|
"1_backlog",
|
||||||
|
Some("Already Named"),
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
);
|
||||||
|
|
||||||
|
migrate_names_from_slugs();
|
||||||
|
|
||||||
|
let after = read_item("43_story_named_item").unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
after.name.as_deref(),
|
||||||
|
Some("Already Named"),
|
||||||
|
"pre-existing name must not be overwritten"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn migrate_names_from_slugs_noop_when_crdt_not_initialised() {
|
||||||
|
// Should not panic when called before init.
|
||||||
|
// In practice get_crdt() returns None in a fresh thread.
|
||||||
|
// We call it here just to confirm no panic.
|
||||||
|
migrate_names_from_slugs();
|
||||||
|
}
|
||||||
use bft_json_crdt::json_crdt::OpState;
|
use bft_json_crdt::json_crdt::OpState;
|
||||||
use bft_json_crdt::keypair::make_keypair;
|
use bft_json_crdt::keypair::make_keypair;
|
||||||
use bft_json_crdt::op::ROOT_ID;
|
use bft_json_crdt::op::ROOT_ID;
|
||||||
|
|||||||
@@ -241,10 +241,13 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String
|
|||||||
let user_story = args.get("user_story").and_then(|v| v.as_str());
|
let user_story = args.get("user_story").and_then(|v| v.as_str());
|
||||||
let description = args.get("description").and_then(|v| v.as_str());
|
let description = args.get("description").and_then(|v| v.as_str());
|
||||||
|
|
||||||
// Collect front matter fields: explicit `agent` param + arbitrary `front_matter` object.
|
// Collect front matter fields: explicit `name`/`agent` params + arbitrary `front_matter` object.
|
||||||
// Values are passed as serde_json::Value so native booleans, numbers, and arrays are
|
// Values are passed as serde_json::Value so native booleans, numbers, and arrays are
|
||||||
// preserved and encoded correctly as unquoted YAML by update_story_in_file.
|
// preserved and encoded correctly as unquoted YAML by update_story_in_file.
|
||||||
let mut front_matter: HashMap<String, Value> = HashMap::new();
|
let mut front_matter: HashMap<String, Value> = HashMap::new();
|
||||||
|
if let Some(name) = args.get("name").and_then(|v| v.as_str()) {
|
||||||
|
front_matter.insert("name".to_string(), Value::String(name.to_string()));
|
||||||
|
}
|
||||||
if let Some(agent) = args.get("agent").and_then(|v| v.as_str()) {
|
if let Some(agent) = args.get("agent").and_then(|v| v.as_str()) {
|
||||||
front_matter.insert("agent".to_string(), Value::String(agent.to_string()));
|
front_matter.insert("agent".to_string(), Value::String(agent.to_string()));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -398,7 +398,7 @@ pub(super) fn handle_tools_list(id: Option<Value>) -> JsonRpcResponse {
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name": "update_story",
|
"name": "update_story",
|
||||||
"description": "Update an existing story file. Can replace the '## User Story' and/or '## Description' section content, and/or set YAML front matter fields (e.g. agent, qa). Auto-commits via the filesystem watcher.",
|
"description": "Update an existing story file. Can rename the story, replace the '## User Story' and/or '## Description' section content, and/or set YAML front matter fields (e.g. agent, qa). Auto-commits via the filesystem watcher.",
|
||||||
"inputSchema": {
|
"inputSchema": {
|
||||||
"type": "object",
|
"type": "object",
|
||||||
"properties": {
|
"properties": {
|
||||||
@@ -406,6 +406,10 @@ pub(super) fn handle_tools_list(id: Option<Value>) -> JsonRpcResponse {
|
|||||||
"type": "string",
|
"type": "string",
|
||||||
"description": "Story identifier (filename stem, e.g. '28_my_story')"
|
"description": "Story identifier (filename stem, e.g. '28_my_story')"
|
||||||
},
|
},
|
||||||
|
"name": {
|
||||||
|
"type": "string",
|
||||||
|
"description": "New human-readable name for the story (stored as a CRDT field; does not change the story_id or any references)"
|
||||||
|
},
|
||||||
"user_story": {
|
"user_story": {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
"description": "New user story text to replace the '## User Story' section content"
|
"description": "New user story text to replace the '## User Story' section content"
|
||||||
|
|||||||
@@ -267,6 +267,10 @@ async fn main() -> Result<(), std::io::Error> {
|
|||||||
}
|
}
|
||||||
if let Err(e) = crdt_state::init(db_path).await {
|
if let Err(e) = crdt_state::init(db_path).await {
|
||||||
slog!("[crdt] Failed to initialise CRDT state layer: {e}");
|
slog!("[crdt] Failed to initialise CRDT state layer: {e}");
|
||||||
|
} else {
|
||||||
|
// Migrate items that have an empty name field: derive the name
|
||||||
|
// from the story ID slug. No-op for items that already have a name.
|
||||||
|
crdt_state::migrate_names_from_slugs();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user