huskies: merge 1158 story Reject boilerplate user stories at save time
This commit is contained in:
@@ -111,10 +111,9 @@ fn parse_pub_item(line: &str) -> Option<(String, String)> {
|
|||||||
let rest = if let Some(r) = trimmed.strip_prefix("pub(") {
|
let rest = if let Some(r) = trimmed.strip_prefix("pub(") {
|
||||||
let end = r.find(')')?;
|
let end = r.find(')')?;
|
||||||
r[end + 1..].trim_start()
|
r[end + 1..].trim_start()
|
||||||
} else if let Some(r) = trimmed.strip_prefix("pub ") {
|
|
||||||
r.trim_start()
|
|
||||||
} else {
|
} else {
|
||||||
return None;
|
let r = trimmed.strip_prefix("pub ")?;
|
||||||
|
r.trim_start()
|
||||||
};
|
};
|
||||||
|
|
||||||
// Handle "async fn"
|
// Handle "async fn"
|
||||||
@@ -139,10 +138,9 @@ fn parse_pub_item(line: &str) -> Option<(String, String)> {
|
|||||||
("const", r.trim_start())
|
("const", r.trim_start())
|
||||||
} else if let Some(r) = rest.strip_prefix("static ") {
|
} else if let Some(r) = rest.strip_prefix("static ") {
|
||||||
("static", r.trim_start())
|
("static", r.trim_start())
|
||||||
} else if let Some(r) = rest.strip_prefix("mod ") {
|
|
||||||
("mod", r.trim_start())
|
|
||||||
} else {
|
} else {
|
||||||
return None;
|
let r = rest.strip_prefix("mod ")?;
|
||||||
|
("mod", r.trim_start())
|
||||||
};
|
};
|
||||||
|
|
||||||
let name: String = name_part
|
let name: String = name_part
|
||||||
|
|||||||
@@ -123,10 +123,9 @@ fn parse_exported_item(line: &str) -> Option<(String, String)> {
|
|||||||
// Strip "export default" or "export"
|
// Strip "export default" or "export"
|
||||||
let rest = if let Some(r) = trimmed.strip_prefix("export default ") {
|
let rest = if let Some(r) = trimmed.strip_prefix("export default ") {
|
||||||
r.trim_start()
|
r.trim_start()
|
||||||
} else if let Some(r) = trimmed.strip_prefix("export ") {
|
|
||||||
r.trim_start()
|
|
||||||
} else {
|
} else {
|
||||||
return None;
|
let r = trimmed.strip_prefix("export ")?;
|
||||||
|
r.trim_start()
|
||||||
};
|
};
|
||||||
|
|
||||||
// Strip optional "async"
|
// Strip optional "async"
|
||||||
@@ -148,10 +147,9 @@ fn parse_exported_item(line: &str) -> Option<(String, String)> {
|
|||||||
("const", r.trim_start())
|
("const", r.trim_start())
|
||||||
} else if let Some(r) = rest.strip_prefix("let ") {
|
} else if let Some(r) = rest.strip_prefix("let ") {
|
||||||
("let", r.trim_start())
|
("let", r.trim_start())
|
||||||
} else if let Some(r) = rest.strip_prefix("enum ") {
|
|
||||||
("enum", r.trim_start())
|
|
||||||
} else {
|
} else {
|
||||||
return None;
|
let r = rest.strip_prefix("enum ")?;
|
||||||
|
("enum", r.trim_start())
|
||||||
};
|
};
|
||||||
|
|
||||||
let name: String = name_part
|
let name: String = name_part
|
||||||
|
|||||||
@@ -51,10 +51,9 @@ pub fn extract_project_rebuild_command(
|
|||||||
.trim()
|
.trim()
|
||||||
.trim_start_matches(|c: char| !c.is_alphanumeric());
|
.trim_start_matches(|c: char| !c.is_alphanumeric());
|
||||||
|
|
||||||
let rest = if let Some(r) = trimmed.strip_prefix("project-rebuild") {
|
let rest = {
|
||||||
|
let r = trimmed.strip_prefix("project-rebuild")?;
|
||||||
r.trim()
|
r.trim()
|
||||||
} else {
|
|
||||||
return None;
|
|
||||||
};
|
};
|
||||||
|
|
||||||
let mut parts = rest.split_whitespace();
|
let mut parts = rest.split_whitespace();
|
||||||
|
|||||||
@@ -156,6 +156,20 @@ mod tests {
|
|||||||
assert_eq!(state.steps[1].status, StepStatus::Generating);
|
assert_eq!(state.steps[1].status, StepStatus::Generating);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn wizard_generate_rejects_boilerplate_user_story_content() {
|
||||||
|
let dir = TempDir::new().unwrap();
|
||||||
|
let ctx = setup(&dir);
|
||||||
|
let result = tool_wizard_generate(
|
||||||
|
&serde_json::json!({"content": "As a user, I want X, so that Y."}),
|
||||||
|
&ctx,
|
||||||
|
);
|
||||||
|
let err = result.unwrap_err();
|
||||||
|
assert!(err.contains("Invalid content"));
|
||||||
|
let state = WizardState::load(dir.path()).unwrap();
|
||||||
|
assert_eq!(state.steps[1].status, StepStatus::Pending);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn wizard_generate_with_content_stages_content() {
|
fn wizard_generate_with_content_stages_content() {
|
||||||
let dir = TempDir::new().unwrap();
|
let dir = TempDir::new().unwrap();
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ pub(crate) mod io;
|
|||||||
pub mod state_machine;
|
pub mod state_machine;
|
||||||
|
|
||||||
use crate::io::wizard::{StepStatus, WizardState, WizardStep, format_wizard_state};
|
use crate::io::wizard::{StepStatus, WizardState, WizardStep, format_wizard_state};
|
||||||
|
use crate::validation::check_boilerplate_user_story;
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
|
|
||||||
// ── Error type ────────────────────────────────────────────────────────────────
|
// ── Error type ────────────────────────────────────────────────────────────────
|
||||||
@@ -21,6 +22,7 @@ use std::path::Path;
|
|||||||
/// - [`Error::NotActive`] → 404 Not Found
|
/// - [`Error::NotActive`] → 404 Not Found
|
||||||
/// - [`Error::InvalidStateTransition`] → 400 Bad Request
|
/// - [`Error::InvalidStateTransition`] → 400 Bad Request
|
||||||
/// - [`Error::MissingInput`] → 400 Bad Request
|
/// - [`Error::MissingInput`] → 400 Bad Request
|
||||||
|
/// - [`Error::InvalidContent`] → 400 Bad Request
|
||||||
/// - [`Error::GenerationFailure`] → 500 Internal Server Error
|
/// - [`Error::GenerationFailure`] → 500 Internal Server Error
|
||||||
/// - [`Error::PersistenceFailure`] → 500 Internal Server Error
|
/// - [`Error::PersistenceFailure`] → 500 Internal Server Error
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
@@ -32,6 +34,8 @@ pub enum Error {
|
|||||||
InvalidStateTransition(String),
|
InvalidStateTransition(String),
|
||||||
/// Required input was absent (e.g. content not staged before confirming).
|
/// Required input was absent (e.g. content not staged before confirming).
|
||||||
MissingInput(String),
|
MissingInput(String),
|
||||||
|
/// Staged content failed validation (e.g. boilerplate user-story template).
|
||||||
|
InvalidContent(String),
|
||||||
/// The LLM or agent failed to generate content for the step.
|
/// The LLM or agent failed to generate content for the step.
|
||||||
GenerationFailure(String),
|
GenerationFailure(String),
|
||||||
/// A filesystem read or write operation failed.
|
/// A filesystem read or write operation failed.
|
||||||
@@ -46,6 +50,7 @@ impl std::fmt::Display for Error {
|
|||||||
}
|
}
|
||||||
Self::InvalidStateTransition(msg) => write!(f, "Invalid state transition: {msg}"),
|
Self::InvalidStateTransition(msg) => write!(f, "Invalid state transition: {msg}"),
|
||||||
Self::MissingInput(msg) => write!(f, "Missing input: {msg}"),
|
Self::MissingInput(msg) => write!(f, "Missing input: {msg}"),
|
||||||
|
Self::InvalidContent(msg) => write!(f, "Invalid content: {msg}"),
|
||||||
Self::GenerationFailure(msg) => write!(f, "Generation failed: {msg}"),
|
Self::GenerationFailure(msg) => write!(f, "Generation failed: {msg}"),
|
||||||
Self::PersistenceFailure(msg) => write!(f, "Persistence error: {msg}"),
|
Self::PersistenceFailure(msg) => write!(f, "Persistence error: {msg}"),
|
||||||
}
|
}
|
||||||
@@ -104,6 +109,8 @@ pub fn status(root: &Path) -> Result<String, Error> {
|
|||||||
///
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
/// - [`Error::NotActive`] if no wizard is active.
|
/// - [`Error::NotActive`] if no wizard is active.
|
||||||
|
/// - [`Error::InvalidContent`] if `content` matches the boilerplate
|
||||||
|
/// "As a …, I want …, so that …" user-story template.
|
||||||
/// - [`Error::PersistenceFailure`] if saving state fails.
|
/// - [`Error::PersistenceFailure`] if saving state fails.
|
||||||
pub fn generate(root: &Path, content: Option<&str>) -> Result<String, Error> {
|
pub fn generate(root: &Path, content: Option<&str>) -> Result<String, Error> {
|
||||||
let mut state = io::load(root).ok_or(Error::NotActive)?;
|
let mut state = io::load(root).ok_or(Error::NotActive)?;
|
||||||
@@ -116,6 +123,9 @@ pub fn generate(root: &Path, content: Option<&str>) -> Result<String, Error> {
|
|||||||
let step = state.steps[current_idx].step;
|
let step = state.steps[current_idx].step;
|
||||||
|
|
||||||
if let Some(c) = content {
|
if let Some(c) = content {
|
||||||
|
if let Some(err) = check_boilerplate_user_story("content", c) {
|
||||||
|
return Err(Error::InvalidContent(err.to_string()));
|
||||||
|
}
|
||||||
state.set_step_status(step, StepStatus::AwaitingConfirmation, Some(c.to_string()));
|
state.set_step_status(step, StepStatus::AwaitingConfirmation, Some(c.to_string()));
|
||||||
io::save(&state, root)?;
|
io::save(&state, root)?;
|
||||||
return Ok(format!(
|
return Ok(format!(
|
||||||
@@ -381,6 +391,21 @@ mod tests {
|
|||||||
assert_eq!(state.steps[1].status, StepStatus::AwaitingConfirmation);
|
assert_eq!(state.steps[1].status, StepStatus::AwaitingConfirmation);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn generate_rejects_boilerplate_user_story_content() {
|
||||||
|
let dir = TempDir::new().unwrap();
|
||||||
|
init_wizard(&dir);
|
||||||
|
let err = generate(
|
||||||
|
dir.path(),
|
||||||
|
Some("As a user, I want this, so that I get value."),
|
||||||
|
)
|
||||||
|
.unwrap_err();
|
||||||
|
assert!(matches!(err, Error::InvalidContent(_)));
|
||||||
|
// Content must not be staged when rejected.
|
||||||
|
let state = get_state(dir.path()).unwrap();
|
||||||
|
assert_eq!(state.steps[1].status, StepStatus::Pending);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn generate_no_content_marks_generating() {
|
fn generate_no_content_marks_generating() {
|
||||||
let dir = TempDir::new().unwrap();
|
let dir = TempDir::new().unwrap();
|
||||||
|
|||||||
@@ -29,6 +29,8 @@ pub enum ValidationError {
|
|||||||
},
|
},
|
||||||
/// A field value contains a tool-call grammar fragment that must be rejected.
|
/// A field value contains a tool-call grammar fragment that must be rejected.
|
||||||
AntiGrammarToken { field: String, token: String },
|
AntiGrammarToken { field: String, token: String },
|
||||||
|
/// A field value matches the unfilled "As a …, I want …, so that …" boilerplate template.
|
||||||
|
BoilerplateUserStory { field: String },
|
||||||
/// A numeric field value is outside its allowed range.
|
/// A numeric field value is outside its allowed range.
|
||||||
OutOfRange {
|
OutOfRange {
|
||||||
field: String,
|
field: String,
|
||||||
@@ -91,6 +93,15 @@ impl fmt::Display for ValidationError {
|
|||||||
"field '{field}' contains a tool-call grammar fragment: {token:?}"
|
"field '{field}' contains a tool-call grammar fragment: {token:?}"
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
Self::BoilerplateUserStory { field } => {
|
||||||
|
write!(
|
||||||
|
f,
|
||||||
|
"field '{field}' looks like the unfilled 'As a …, I want …, so that …' \
|
||||||
|
template. Rephrase it as a concrete user story in plain language, \
|
||||||
|
describing the actual actor, action, and benefit. If you cannot \
|
||||||
|
determine the right phrasing, ask the commissioning user."
|
||||||
|
)
|
||||||
|
}
|
||||||
Self::OutOfRange {
|
Self::OutOfRange {
|
||||||
field,
|
field,
|
||||||
min,
|
min,
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ mod sanitize;
|
|||||||
pub use error::{ValidationError, format_errors_as_json};
|
pub use error::{ValidationError, format_errors_as_json};
|
||||||
pub use newtypes::{
|
pub use newtypes::{
|
||||||
AcceptanceCriterion, DependsOnId, Description, StoryId, StoryName, TargetStage,
|
AcceptanceCriterion, DependsOnId, Description, StoryId, StoryName, TargetStage,
|
||||||
|
check_boilerplate_user_story,
|
||||||
};
|
};
|
||||||
pub use requests::{
|
pub use requests::{
|
||||||
AddCriterionRequest, ConvertItemTypeRequest, CreateBugRequest, CreateEpicRequest,
|
AddCriterionRequest, ConvertItemTypeRequest, CreateBugRequest, CreateEpicRequest,
|
||||||
|
|||||||
@@ -6,7 +6,9 @@
|
|||||||
//! in preference to nutype's lower-level `new()`.
|
//! in preference to nutype's lower-level `new()`.
|
||||||
|
|
||||||
use nutype::nutype;
|
use nutype::nutype;
|
||||||
|
use regex::Regex;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
|
use std::sync::LazyLock;
|
||||||
|
|
||||||
use super::error::ValidationError;
|
use super::error::ValidationError;
|
||||||
use super::sanitize;
|
use super::sanitize;
|
||||||
@@ -51,6 +53,25 @@ fn check_grammar_tokens(field: &str, value: &str) -> Vec<ValidationError> {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Matches the classic "As a …, I want …, so that …" user-story template,
|
||||||
|
/// case-insensitively and tolerant of punctuation/whitespace variation between
|
||||||
|
/// the three clauses (commas, dashes, or nothing at all).
|
||||||
|
static BOILERPLATE_USER_STORY_RE: LazyLock<Regex> =
|
||||||
|
LazyLock::new(|| Regex::new(r"(?is)\bas\s+an?\b.*?\bi\s+want\b.*?\bso\s+that\b").unwrap());
|
||||||
|
|
||||||
|
/// Return a [`ValidationError::BoilerplateUserStory`] if `value` matches the
|
||||||
|
/// unfilled "As a …, I want …, so that …" template rather than a concrete,
|
||||||
|
/// plain-language user story.
|
||||||
|
pub fn check_boilerplate_user_story(field: &str, value: &str) -> Option<ValidationError> {
|
||||||
|
if BOILERPLATE_USER_STORY_RE.is_match(value) {
|
||||||
|
Some(ValidationError::BoilerplateUserStory {
|
||||||
|
field: field.to_string(),
|
||||||
|
})
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// StoryName newtype
|
// StoryName newtype
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@@ -481,6 +502,60 @@ mod tests {
|
|||||||
assert!(matches!(err[0], ValidationError::AntiGrammarToken { .. }));
|
assert!(matches!(err[0], ValidationError::AntiGrammarToken { .. }));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- check_boilerplate_user_story ---
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn boilerplate_user_story_rejects_classic_template() {
|
||||||
|
let err = check_boilerplate_user_story(
|
||||||
|
"user_story",
|
||||||
|
"As a user, I want to log in, so that I can access my account.",
|
||||||
|
);
|
||||||
|
assert!(matches!(
|
||||||
|
err,
|
||||||
|
Some(ValidationError::BoilerplateUserStory { .. })
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn boilerplate_user_story_rejects_without_commas() {
|
||||||
|
let err = check_boilerplate_user_story(
|
||||||
|
"user_story",
|
||||||
|
"As a developer I want CI so that builds are faster",
|
||||||
|
);
|
||||||
|
assert!(matches!(
|
||||||
|
err,
|
||||||
|
Some(ValidationError::BoilerplateUserStory { .. })
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn boilerplate_user_story_rejects_case_insensitively() {
|
||||||
|
let err = check_boilerplate_user_story(
|
||||||
|
"user_story",
|
||||||
|
"AS AN admin I WANT reports SO THAT I can audit usage",
|
||||||
|
);
|
||||||
|
assert!(matches!(
|
||||||
|
err,
|
||||||
|
Some(ValidationError::BoilerplateUserStory { .. })
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn boilerplate_user_story_accepts_concrete_plain_language() {
|
||||||
|
let err = check_boilerplate_user_story(
|
||||||
|
"user_story",
|
||||||
|
"The pipeline dashboard should show test coverage next to each story so \
|
||||||
|
QA doesn't have to check CI separately.",
|
||||||
|
);
|
||||||
|
assert!(err.is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn boilerplate_user_story_accepts_partial_phrase_without_so_that() {
|
||||||
|
let err = check_boilerplate_user_story("user_story", "As a user I want this");
|
||||||
|
assert!(err.is_none());
|
||||||
|
}
|
||||||
|
|
||||||
// --- DependsOnId ---
|
// --- DependsOnId ---
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ use serde_json::Value;
|
|||||||
use super::error::{ValidationError, format_errors_as_json};
|
use super::error::{ValidationError, format_errors_as_json};
|
||||||
use super::newtypes::{
|
use super::newtypes::{
|
||||||
AcceptanceCriterion, DependsOnId, Description, StoryId, StoryName, TargetStage,
|
AcceptanceCriterion, DependsOnId, Description, StoryId, StoryName, TargetStage,
|
||||||
|
check_boilerplate_user_story,
|
||||||
};
|
};
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@@ -91,7 +92,16 @@ impl CreateStoryRequest {
|
|||||||
let user_story = match args.get("user_story").and_then(|v| v.as_str()) {
|
let user_story = match args.get("user_story").and_then(|v| v.as_str()) {
|
||||||
None => None,
|
None => None,
|
||||||
Some(raw) => match Description::parse("user_story", raw) {
|
Some(raw) => match Description::parse("user_story", raw) {
|
||||||
Ok(d) => Some(d),
|
Ok(d) => {
|
||||||
|
if let Some(boilerplate_err) =
|
||||||
|
check_boilerplate_user_story("user_story", d.as_str())
|
||||||
|
{
|
||||||
|
errors.push(boilerplate_err);
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
Some(d)
|
||||||
|
}
|
||||||
|
}
|
||||||
Err(mut errs) => {
|
Err(mut errs) => {
|
||||||
errors.append(&mut errs);
|
errors.append(&mut errs);
|
||||||
None
|
None
|
||||||
@@ -1365,6 +1375,17 @@ mod tests {
|
|||||||
assert!(err.contains("AntiGrammarToken"));
|
assert!(err.contains("AntiGrammarToken"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn create_story_request_rejects_boilerplate_user_story() {
|
||||||
|
let args = json!({
|
||||||
|
"name": "Valid Name",
|
||||||
|
"user_story": "As a user, I want to do a thing, so that I get value.",
|
||||||
|
"acceptance_criteria": ["AC1"]
|
||||||
|
});
|
||||||
|
let err = CreateStoryRequest::from_json(&args).unwrap_err();
|
||||||
|
assert!(err.contains("BoilerplateUserStory"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn create_story_request_with_all_optional_fields() {
|
fn create_story_request_with_all_optional_fields() {
|
||||||
let args = json!({
|
let args = json!({
|
||||||
|
|||||||
Reference in New Issue
Block a user