Skip to content

Commit 6216878

Browse files
authored
Merge pull request #48 from CurtisTechSolutions/feat/analysis-variations
feat(analysis): expose ranked MultiPV recommendations
2 parents 4f2c290 + 05c2899 commit 6216878

10 files changed

Lines changed: 321 additions & 42 deletions

File tree

‎crates/apps/gfa-server/src/stockfish_tests.rs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,7 @@ async fn stockfish_http_seats_analysis_and_idempotency() -> Result<(), ServerErr
103103
.contains(&analysis["best_moves"][0]));
104104
assert!(analysis["evaluation"].is_null());
105105
assert_eq!(
106-
analysis["advice"]["details"]["variations"]
107-
.as_array()
108-
.ok_or("variations")?
109-
.len(),
106+
analysis["variations"].as_array().ok_or("variations")?.len(),
110107
2
111108
);
112109
let (_, after) = call(&app.router, "GET", &replay_path, Value::Null, None).await?;

‎crates/apps/gfa-server/src/uci_tests.rs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,55 @@ fn multipv_retains_only_legal_lines_and_selected_move_diagnostics() -> TestResul
141141
);
142142
Ok(())
143143
}
144+
145+
#[test]
146+
fn analysis_ranks_all_variations_and_rejects_duplicate_roots() -> TestResult {
147+
let game = game()?;
148+
let state = game.initial_state(&json!({}), 7)?;
149+
let observation = game.observe(&state, Viewer::Player(0))?;
150+
let legal = game.legal_actions(&state, 0)?;
151+
let turn = PlayerTurn {
152+
seat: 0,
153+
observation: &observation,
154+
legal_actions: &legal,
155+
};
156+
let limits = SearchLimits {
157+
nodes: 1000,
158+
depth: 8,
159+
time_ms: 5000,
160+
seed: 7,
161+
};
162+
let mut output = result();
163+
let mut second = output.variations[0].clone();
164+
second.multipv = Some(2);
165+
second.pv = vec!["d2d4".into(), "d7d5".into()];
166+
output.variations.insert(0, second);
167+
let choices =
168+
gfa_engine_uci::validate_analysis(game.as_ref(), &json!({}), &turn, limits, &output)?;
169+
assert_eq!(
170+
choices
171+
.iter()
172+
.map(|choice| choice.action.string.as_str())
173+
.collect::<Vec<_>>(),
174+
["e2e4", "d2d4"]
175+
);
176+
for choice in choices {
177+
assert_eq!(
178+
choice.info.principal_variation.first(),
179+
Some(&choice.action.string)
180+
);
181+
assert_eq!(
182+
choice.info.advice.ok_or("diagnostics")?.details["variations"]
183+
.as_array()
184+
.ok_or("line")?
185+
.len(),
186+
1
187+
);
188+
}
189+
output.variations[0].pv = output.variations[1].pv.clone();
190+
assert_eq!(
191+
gfa_engine_uci::validate_analysis(game.as_ref(), &json!({}), &turn, limits, &output),
192+
Err(OpponentError::InvalidResponse)
193+
);
194+
Ok(())
195+
}

‎crates/apps/gfa-server/src/workers.rs‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,13 @@ impl gfa_opponents::Clock for SearchClock {
3636
}
3737
}
3838

39-
impl OpponentExecutor for Workers {
40-
fn execute(&self, job: OpponentJob) -> OpponentFuture<'_> {
39+
impl Workers {
40+
fn schedule<T: Send + 'static>(
41+
&self,
42+
job: impl FnOnce(&SearchClock) -> Result<T, gfa_api_types::ApiError> + Send + 'static,
43+
) -> std::pin::Pin<
44+
Box<dyn std::future::Future<Output = Result<T, gfa_api_types::ApiError>> + Send>,
45+
> {
4146
let capacity = self.0.clone();
4247
Box::pin(async move {
4348
let permit = capacity.try_acquire_owned().map_err(|_| {
@@ -50,9 +55,9 @@ impl OpponentExecutor for Workers {
5055
let cancelled = Arc::new(AtomicBool::new(false));
5156
let _cancel_on_drop = CancelOnDrop(cancelled.clone());
5257
tokio::task::spawn_blocking(move || {
53-
// A dropped HTTP request must not release capacity while CPU work continues.
58+
// Capacity remains leased until CPU work responds to cancellation and exits.
5459
let _permit = permit;
55-
job.run(&SearchClock {
60+
job(&SearchClock {
5661
cancelled,
5762
clock: gfa_opponents::SystemClock::default(),
5863
})
@@ -68,6 +73,14 @@ impl OpponentExecutor for Workers {
6873
})
6974
}
7075
}
76+
impl OpponentExecutor for Workers {
77+
fn execute(&self, job: OpponentJob) -> OpponentFuture<'_> {
78+
self.schedule(move |clock| job.run(clock))
79+
}
80+
fn analyze(&self, job: OpponentJob) -> gfa_service::AnalysisFuture<'_> {
81+
self.schedule(move |clock| job.analyze(clock))
82+
}
83+
}
7184

7285
#[cfg(test)]
7386
mod tests {

‎crates/core/gfa-core/src/player.rs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ impl SearchLimits {
6363
}
6464

6565
/// Search diagnostics accompanying a chosen move.
66-
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
66+
#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))]
67+
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, crate::schemars::JsonSchema)]
6768

6869
pub struct ChoiceInfo {
6970
/// Optional engine-provided solving explanation.
@@ -84,7 +85,8 @@ pub struct ChoiceInfo {
8485
}
8586

8687
/// One legal move and optional explanatory diagnostics.
87-
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
88+
#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))]
89+
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, crate::schemars::JsonSchema)]
8890

8991
pub struct ActionChoice {
9092
/// Canonical action selected from the supplied legal list.
@@ -141,6 +143,17 @@ pub trait Opponent: Send + Sync {
141143
self.choose_action(turn, limits, clock)
142144
.map_err(OpponentError::Game)
143145
}
146+
147+
/// Ranked legal analysis recommendations. Existing players return their single decision.
148+
/// Hosts validate the returned list and enforce a maximum of sixteen alternatives.
149+
fn analyze(
150+
&self,
151+
turn: &PlayerTurn<'_>,
152+
limits: SearchLimits,
153+
clock: &dyn Clock,
154+
) -> Result<Vec<ActionChoice>, OpponentError> {
155+
self.decide(turn, limits, clock).map(|choice| vec![choice])
156+
}
144157
}
145158

146159
fn invalid(message: &str) -> GameError {

‎crates/domain/gfa-api-types/src/opponents.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,9 @@ pub struct AnalysisResult {
116116
pub seed: u64,
117117
/// Recommended legal actions in order of preference.
118118
pub best_moves: Vec<LegalAction>,
119+
/// Per-move analysis in the same preference order as best_moves.
120+
#[serde(default)]
121+
pub variations: Vec<gfa_core::ActionChoice>,
119122
/// Expected return for the requesting seat, absent without completed search.
120123
pub evaluation: Option<f64>,
121124
/// Canonical continuation, beginning with the recommended action.

‎crates/domain/gfa-service/src/lib.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ mod fork;
2727
mod opponents;
2828
mod planning;
2929
pub use opponents::{
30-
BuiltinOpponentFactory, OpponentExecutor, OpponentFactory, OpponentFuture, OpponentJob,
30+
AnalysisFuture, BuiltinOpponentFactory, OpponentExecutor, OpponentFactory, OpponentFuture,
31+
OpponentJob,
3132
};
3233

3334
mod seats;

‎crates/domain/gfa-service/src/opponents.rs‎

Lines changed: 127 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ use std::{future::Future, pin::Pin, sync::Arc};
1515
pub type OpponentFuture<'a> =
1616
Pin<Box<dyn Future<Output = Result<ActionChoice, ApiError>> + Send + 'a>>;
1717

18+
/// Bounded recommendations returned by an analysis worker.
19+
pub type AnalysisFuture<'a> =
20+
Pin<Box<dyn Future<Output = Result<Vec<ActionChoice>, ApiError>> + Send + 'a>>;
21+
1822
/// Select installed players without exposing live state or transport details.
1923
pub trait OpponentFactory: Send + Sync {
2024
/// Stable catalog for this game.
@@ -33,6 +37,10 @@ pub trait OpponentFactory: Send + Sync {
3337
pub trait OpponentExecutor: Send + Sync {
3438
/// Run outside the request executor, rejecting excess work instead of queueing without bounds.
3539
fn execute(&self, job: OpponentJob) -> OpponentFuture<'_>;
40+
/// Analyze on the host's bounded executor. Legacy executors retain single-choice behavior.
41+
fn analyze(&self, job: OpponentJob) -> AnalysisFuture<'_> {
42+
Box::pin(async move { self.execute(job).await.map(|choice| vec![choice]) })
43+
}
3644
}
3745

3846
/// Owned planning input. No raw match state or match RNG is passed to the worker.
@@ -45,6 +53,21 @@ pub struct OpponentJob {
4553
}
4654

4755
impl OpponentJob {
56+
/// Request ranked analysis recommendations with the same observation-only input.
57+
pub fn analyze(self, clock: &dyn Clock) -> Result<Vec<ActionChoice>, ApiError> {
58+
self.opponent
59+
.analyze(
60+
&PlayerTurn {
61+
seat: self.seat,
62+
observation: &self.observation,
63+
legal_actions: &self.legal_actions,
64+
},
65+
self.limits,
66+
clock,
67+
)
68+
.map_err(error::opponent)
69+
}
70+
4871
/// Execute with an injected monotonic clock. The host supplies the runtime.
4972
pub fn run(self, clock: &dyn Clock) -> Result<ActionChoice, ApiError> {
5073
self.opponent
@@ -267,27 +290,42 @@ impl GameService {
267290
"Select the current player or an earlier turn.",
268291
));
269292
}
270-
let choice = self
271-
.choose_opponent(
272-
planning.game,
273-
planning.config,
274-
&request.opponent,
275-
seed,
276-
seat,
277-
visible,
293+
let (factory, executor) = self.opponents.as_ref().ok_or_else(|| {
294+
ApiError::new(
295+
"ENGINE_UNAVAILABLE",
296+
"No opponent executor is installed",
297+
"Configure an opponent worker pool on the host.",
278298
)
299+
})?;
300+
let (opponent, limits) =
301+
factory.create(planning.game, planning.config, &request.opponent, seed)?;
302+
let choices = executor
303+
.analyze(OpponentJob {
304+
opponent,
305+
seat,
306+
observation: visible.observation,
307+
legal_actions: visible.legal_actions.clone(),
308+
limits,
309+
})
279310
.await?;
311+
validate_analysis_choices(&choices, &visible.legal_actions)?;
312+
let choice = choices.first().ok_or_else(invalid_analysis)?;
280313
Ok(AnalysisResult {
281-
advice: choice.info.advice,
314+
advice: choice.info.advice.clone(),
282315
game_id: request.game_id,
283316
opponent: request.opponent.id,
284317
seed,
285-
best_moves: vec![choice.action],
318+
best_moves: choices.iter().map(|choice| choice.action.clone()).collect(),
286319
evaluation: choice.info.evaluation,
287-
principal_variation: choice.info.principal_variation,
288-
nodes: choice.info.nodes,
320+
principal_variation: choice.info.principal_variation.clone(),
321+
nodes: choices
322+
.iter()
323+
.map(|choice| choice.info.nodes)
324+
.max()
325+
.unwrap_or(0),
289326
depth: choice.info.depth,
290-
budget_exhausted: choice.info.budget_exhausted,
327+
budget_exhausted: choices.iter().any(|choice| choice.info.budget_exhausted),
328+
variations: choices,
291329
})
292330
}
293331
}
@@ -339,3 +377,79 @@ mod failure_tests {
339377
Ok(())
340378
}
341379
}
380+
381+
fn invalid_analysis() -> ApiError {
382+
ApiError::new(
383+
"ENGINE_INVALID_RESPONSE",
384+
"Opponent returned invalid analysis recommendations",
385+
"Select another opponent and report the provider failure.",
386+
)
387+
}
388+
fn validate_analysis_choices(
389+
choices: &[ActionChoice],
390+
legal: &[LegalAction],
391+
) -> Result<(), ApiError> {
392+
if choices.is_empty() || choices.len() > 16 {
393+
return Err(invalid_analysis());
394+
}
395+
let mut seen = std::collections::BTreeSet::new();
396+
for choice in choices {
397+
if !legal.contains(&choice.action)
398+
|| !seen.insert(choice.action.index)
399+
|| choice
400+
.info
401+
.evaluation
402+
.is_some_and(|score| !score.is_finite())
403+
|| choice.info.principal_variation.len() > 128
404+
|| choice
405+
.info
406+
.principal_variation
407+
.first()
408+
.is_some_and(|first| first != &choice.action.string)
409+
{
410+
return Err(invalid_analysis());
411+
}
412+
}
413+
Ok(())
414+
}
415+
416+
#[cfg(test)]
417+
mod analysis_validation_tests {
418+
use super::*;
419+
fn choice() -> ActionChoice {
420+
ActionChoice {
421+
action: LegalAction {
422+
string: "a".into(),
423+
json: serde_json::json!("a"),
424+
index: 0,
425+
},
426+
info: gfa_core::ChoiceInfo {
427+
advice: None,
428+
algorithm: "test".into(),
429+
nodes: 1,
430+
depth: 1,
431+
evaluation: Some(0.0),
432+
principal_variation: vec!["a".into()],
433+
budget_exhausted: false,
434+
},
435+
}
436+
}
437+
#[test]
438+
fn rejects_empty_duplicate_nonfinite_illegal_and_mismatched_analysis() {
439+
let first = choice();
440+
let legal = vec![first.action.clone()];
441+
assert!(validate_analysis_choices(std::slice::from_ref(&first), &legal).is_ok());
442+
assert!(validate_analysis_choices(&[], &legal).is_err());
443+
assert!(validate_analysis_choices(&vec![first.clone(); 17], &legal).is_err());
444+
assert!(validate_analysis_choices(&[first.clone(), first.clone()], &legal).is_err());
445+
let mut bad = first.clone();
446+
bad.info.evaluation = Some(f64::NAN);
447+
assert!(validate_analysis_choices(&[bad], &legal).is_err());
448+
let mut bad = first.clone();
449+
bad.action.string = "b".into();
450+
assert!(validate_analysis_choices(&[bad], &legal).is_err());
451+
let mut bad = first;
452+
bad.info.principal_variation[0] = "b".into();
453+
assert!(validate_analysis_choices(&[bad], &legal).is_err());
454+
}
455+
}

‎crates/players/gfa-engine-uci/README.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,11 @@ a result escapes the adapter. PVs expose at most 32 validated plies and mark
4545
truncation. Engine scores retain their units and root-seat perspective in
4646
advice.details; ChoiceInfo.evaluation remains absent because a centipawn or mate
4747
score is not an expected game return. All MultiPV lines are retained there.
48+
49+
50+
The shared Opponent::analyze path returns a ranked ActionChoice for each validated
51+
MultiPV line. The HTTP response includes these in variations and mirrors their
52+
actions in best_moves. Each variation carries its own depth, continuation and
53+
typed score metadata. The default implementation keeps single-choice players
54+
compatible. Analysis uses the same bounded worker capacity and cancellation
55+
policy as automatic moves.

‎crates/players/gfa-engine-uci/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,4 +197,4 @@ pub(crate) fn invalid(message: &str) -> GameError {
197197
mod tests;
198198

199199
mod player;
200-
pub use player::{validate_recommendation, UciOpponent};
200+
pub use player::{validate_analysis, validate_recommendation, UciOpponent};

0 commit comments

Comments
 (0)