prompt: preserve feature instruction order
This commit is contained in:
parent
44e7014d83
commit
450e0cddbd
|
|
@ -11,7 +11,7 @@
|
||||||
//! [`crate::hook::HookRegistryBuilder`], and provider output is represented as
|
//! [`crate::hook::HookRegistryBuilder`], and provider output is represented as
|
||||||
//! ordinary feature reports/diagnostics instead of a separate authority layer.
|
//! ordinary feature reports/diagnostics instead of a separate authority layer.
|
||||||
|
|
||||||
use std::collections::{BTreeMap, HashMap, HashSet};
|
use std::collections::{HashMap, HashSet};
|
||||||
use std::fmt;
|
use std::fmt;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
|
|
@ -340,18 +340,10 @@ impl fmt::Display for FeatureInstructionId {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)]
|
|
||||||
#[serde(rename_all = "snake_case")]
|
|
||||||
pub enum FeatureInstructionOrder {
|
|
||||||
OrchestrationPolicy,
|
|
||||||
WorkflowPolicy,
|
|
||||||
}
|
|
||||||
|
|
||||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
pub struct FeatureInstructionDeclaration {
|
pub struct FeatureInstructionDeclaration {
|
||||||
pub id: FeatureInstructionId,
|
pub id: FeatureInstructionId,
|
||||||
pub prompt_ref: String,
|
pub prompt_ref: String,
|
||||||
pub order: FeatureInstructionOrder,
|
|
||||||
pub description: String,
|
pub description: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -359,7 +351,6 @@ impl FeatureInstructionDeclaration {
|
||||||
pub fn new(
|
pub fn new(
|
||||||
id: FeatureInstructionId,
|
id: FeatureInstructionId,
|
||||||
prompt_ref: impl Into<String>,
|
prompt_ref: impl Into<String>,
|
||||||
order: FeatureInstructionOrder,
|
|
||||||
description: impl Into<String>,
|
description: impl Into<String>,
|
||||||
) -> Result<Self, FeatureInstallError> {
|
) -> Result<Self, FeatureInstallError> {
|
||||||
let prompt_ref = prompt_ref.into();
|
let prompt_ref = prompt_ref.into();
|
||||||
|
|
@ -371,7 +362,6 @@ impl FeatureInstructionDeclaration {
|
||||||
Ok(Self {
|
Ok(Self {
|
||||||
id,
|
id,
|
||||||
prompt_ref,
|
prompt_ref,
|
||||||
order,
|
|
||||||
description: description.into(),
|
description: description.into(),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
@ -1396,13 +1386,14 @@ impl FeatureRegistryInstallReport {
|
||||||
pub fn dedupe_instruction_contributions(
|
pub fn dedupe_instruction_contributions(
|
||||||
instructions: impl IntoIterator<Item = FeatureInstructionDeclaration>,
|
instructions: impl IntoIterator<Item = FeatureInstructionDeclaration>,
|
||||||
) -> Vec<FeatureInstructionDeclaration> {
|
) -> Vec<FeatureInstructionDeclaration> {
|
||||||
let mut by_id = BTreeMap::new();
|
let mut seen = HashSet::new();
|
||||||
|
let mut deduped = Vec::new();
|
||||||
for instruction in instructions {
|
for instruction in instructions {
|
||||||
by_id.entry(instruction.id.clone()).or_insert(instruction);
|
if seen.insert(instruction.id.clone()) {
|
||||||
|
deduped.push(instruction);
|
||||||
}
|
}
|
||||||
let mut instructions = by_id.into_values().collect::<Vec<_>>();
|
}
|
||||||
instructions.sort_by(|a, b| a.order.cmp(&b.order).then_with(|| a.id.cmp(&b.id)));
|
deduped
|
||||||
instructions
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Builder/installer for enabled feature modules.
|
/// Builder/installer for enabled feature modules.
|
||||||
|
|
@ -1730,15 +1721,10 @@ mod tests {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn instruction(
|
fn instruction(id: &'static str, prompt_ref: &'static str) -> FeatureInstructionDeclaration {
|
||||||
id: &'static str,
|
|
||||||
prompt_ref: &'static str,
|
|
||||||
order: FeatureInstructionOrder,
|
|
||||||
) -> FeatureInstructionDeclaration {
|
|
||||||
FeatureInstructionDeclaration::new(
|
FeatureInstructionDeclaration::new(
|
||||||
FeatureInstructionId::builtin(id),
|
FeatureInstructionId::builtin(id),
|
||||||
prompt_ref,
|
prompt_ref,
|
||||||
order,
|
|
||||||
"test instruction",
|
"test instruction",
|
||||||
)
|
)
|
||||||
.unwrap()
|
.unwrap()
|
||||||
|
|
@ -1771,38 +1757,22 @@ mod tests {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn instruction_contributions_are_deduped_and_sorted() {
|
fn instruction_contributions_are_deduped_in_registration_order() {
|
||||||
let workflow = instruction(
|
let workflow = instruction("workflow", "$yoi/common/tickets");
|
||||||
"workflow",
|
let orchestration = instruction("orchestration", "$yoi/common/worker-orchestration");
|
||||||
"$yoi/common/tickets",
|
|
||||||
FeatureInstructionOrder::WorkflowPolicy,
|
|
||||||
);
|
|
||||||
let orchestration = instruction(
|
|
||||||
"orchestration",
|
|
||||||
"$yoi/common/worker-orchestration",
|
|
||||||
FeatureInstructionOrder::OrchestrationPolicy,
|
|
||||||
);
|
|
||||||
let contributions = dedupe_instruction_contributions([
|
let contributions = dedupe_instruction_contributions([
|
||||||
workflow.clone(),
|
workflow.clone(),
|
||||||
orchestration.clone(),
|
orchestration.clone(),
|
||||||
workflow.clone(),
|
workflow.clone(),
|
||||||
]);
|
]);
|
||||||
|
|
||||||
assert_eq!(contributions, vec![orchestration, workflow]);
|
assert_eq!(contributions, vec![workflow, orchestration]);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn undeclared_instruction_contribution_is_rejected() {
|
fn undeclared_instruction_contribution_is_rejected() {
|
||||||
let declared = instruction(
|
let declared = instruction("declared", "$yoi/common/tickets");
|
||||||
"declared",
|
let undeclared = instruction("undeclared", "$yoi/common/tickets");
|
||||||
"$yoi/common/tickets",
|
|
||||||
FeatureInstructionOrder::WorkflowPolicy,
|
|
||||||
);
|
|
||||||
let undeclared = instruction(
|
|
||||||
"undeclared",
|
|
||||||
"$yoi/common/tickets",
|
|
||||||
FeatureInstructionOrder::WorkflowPolicy,
|
|
||||||
);
|
|
||||||
let descriptor =
|
let descriptor =
|
||||||
FeatureDescriptor::builtin("instruction", "Instruction").with_instruction(declared);
|
FeatureDescriptor::builtin("instruction", "Instruction").with_instruction(declared);
|
||||||
let mut hook_builder = HookRegistryBuilder::default();
|
let mut hook_builder = HookRegistryBuilder::default();
|
||||||
|
|
|
||||||
|
|
@ -20,7 +20,7 @@ use ticket::{
|
||||||
use crate::feature::{
|
use crate::feature::{
|
||||||
FeatureDescriptor, FeatureDiagnostic, FeatureInstallContext, FeatureInstallError,
|
FeatureDescriptor, FeatureDiagnostic, FeatureInstallContext, FeatureInstallError,
|
||||||
FeatureInstructionContribution, FeatureInstructionDeclaration, FeatureInstructionId,
|
FeatureInstructionContribution, FeatureInstructionDeclaration, FeatureInstructionId,
|
||||||
FeatureInstructionOrder, FeatureModule, ToolContribution, ToolDeclaration,
|
FeatureModule, ToolContribution, ToolDeclaration,
|
||||||
};
|
};
|
||||||
|
|
||||||
const FEATURE_ID: &str = "ticket";
|
const FEATURE_ID: &str = "ticket";
|
||||||
|
|
@ -34,7 +34,6 @@ fn ticket_workflow_instruction() -> FeatureInstructionDeclaration {
|
||||||
FeatureInstructionDeclaration::new(
|
FeatureInstructionDeclaration::new(
|
||||||
FeatureInstructionId::builtin(TICKET_WORKFLOW_INSTRUCTION_ID),
|
FeatureInstructionId::builtin(TICKET_WORKFLOW_INSTRUCTION_ID),
|
||||||
TICKET_WORKFLOW_PROMPT_REF,
|
TICKET_WORKFLOW_PROMPT_REF,
|
||||||
FeatureInstructionOrder::WorkflowPolicy,
|
|
||||||
"Typed Ticket workflow guidance",
|
"Typed Ticket workflow guidance",
|
||||||
)
|
)
|
||||||
.expect("static Ticket workflow instruction declaration is valid")
|
.expect("static Ticket workflow instruction declaration is valid")
|
||||||
|
|
|
||||||
|
|
@ -414,7 +414,6 @@ mod tests {
|
||||||
FeatureInstructionDeclaration::new(
|
FeatureInstructionDeclaration::new(
|
||||||
crate::feature::FeatureInstructionId::builtin("ticket.workflow"),
|
crate::feature::FeatureInstructionId::builtin("ticket.workflow"),
|
||||||
"$yoi/common/tickets",
|
"$yoi/common/tickets",
|
||||||
crate::feature::FeatureInstructionOrder::WorkflowPolicy,
|
|
||||||
"Ticket workflow guidance",
|
"Ticket workflow guidance",
|
||||||
)
|
)
|
||||||
.unwrap()
|
.unwrap()
|
||||||
|
|
@ -424,7 +423,6 @@ mod tests {
|
||||||
FeatureInstructionDeclaration::new(
|
FeatureInstructionDeclaration::new(
|
||||||
crate::feature::FeatureInstructionId::builtin("worker.orchestration"),
|
crate::feature::FeatureInstructionId::builtin("worker.orchestration"),
|
||||||
"$yoi/common/worker-orchestration",
|
"$yoi/common/worker-orchestration",
|
||||||
crate::feature::FeatureInstructionOrder::OrchestrationPolicy,
|
|
||||||
"Worker orchestration guidance",
|
"Worker orchestration guidance",
|
||||||
)
|
)
|
||||||
.unwrap()
|
.unwrap()
|
||||||
|
|
|
||||||
|
|
@ -35,8 +35,8 @@ use crate::feature::builtin::{
|
||||||
SessionExploreFeature, SessionExploreState, TaskFeature, render_extract_input,
|
SessionExploreFeature, SessionExploreState, TaskFeature, render_extract_input,
|
||||||
};
|
};
|
||||||
use crate::feature::{
|
use crate::feature::{
|
||||||
FeatureInstructionDeclaration, FeatureInstructionId, FeatureInstructionOrder,
|
FeatureInstructionDeclaration, FeatureInstructionId, FeatureRegistryBuilder,
|
||||||
FeatureRegistryBuilder, FeatureRegistryInstallReport, dedupe_instruction_contributions,
|
FeatureRegistryInstallReport, dedupe_instruction_contributions,
|
||||||
};
|
};
|
||||||
use crate::hook::{
|
use crate::hook::{
|
||||||
Hook, HookRegistryBuilder, OnAbort, OnPromptSubmit, OnTurnEnd, PostToolCall, PreLlmRequest,
|
Hook, HookRegistryBuilder, OnAbort, OnPromptSubmit, OnTurnEnd, PostToolCall, PreLlmRequest,
|
||||||
|
|
@ -54,7 +54,6 @@ fn worker_orchestration_instruction() -> FeatureInstructionDeclaration {
|
||||||
FeatureInstructionDeclaration::new(
|
FeatureInstructionDeclaration::new(
|
||||||
FeatureInstructionId::builtin(WORKER_ORCHESTRATION_INSTRUCTION_ID),
|
FeatureInstructionId::builtin(WORKER_ORCHESTRATION_INSTRUCTION_ID),
|
||||||
WORKER_ORCHESTRATION_PROMPT_REF,
|
WORKER_ORCHESTRATION_PROMPT_REF,
|
||||||
FeatureInstructionOrder::OrchestrationPolicy,
|
|
||||||
"Worker orchestration guidance",
|
"Worker orchestration guidance",
|
||||||
)
|
)
|
||||||
.expect("static Worker orchestration instruction declaration is valid")
|
.expect("static Worker orchestration instruction declaration is valid")
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user