Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2652,6 +2652,11 @@ mod tests {
message.contains("group restore exceeded its budget with 2 batch(es) durable"),
"{message}"
);
// The same machine-readable claim the batch loop's own timeout
// makes: an importer resuming on `durable_batches` must not
// have to parse it out of the message on this arm alone.
assert_eq!(body["integrity"], "durable_prefix", "{body}");
assert_eq!(body["durable_batches"], 2, "{body}");

let _ = std::fs::remove_dir_all(&dir);
}
Expand Down
30 changes: 21 additions & 9 deletions src/api/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,15 +413,27 @@ pub(super) fn restore_refusal(
// partial group write of its own (`restore_groups` validates the
// whole set before applying any of it).
match &refusal {
RestoreGroupsError::Timeout { .. } => error(
code,
format!(
"group restore exceeded its budget with {batches_landed} batch(es) durable \
(TAGURU_REQUEST_TIMEOUT_SECS tunes this); {}",
refusal.text()
),
started_at,
),
RestoreGroupsError::Timeout { .. } => {
// Same machine-readable claim `import_budget_refusal` makes
// for the batch loop: the fields speak for the BATCHES (all
// durable), which is what a resuming importer keys on — the
// group phase itself stays a re-POST, as the note says.
let (integrity, durable_batches) = stream_integrity(batches_landed, false);
validation_error(
code,
format!(
"group restore exceeded its budget with {batches_landed} batch(es) durable \
(TAGURU_REQUEST_TIMEOUT_SECS tunes this); {}",
refusal.text()
),
RefusalDetail {
integrity: Some(integrity),
durable_batches,
..Default::default()
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
started_at,
)
}
_ => {
let (integrity, durable_batches) = stream_integrity(batches_landed, false);
validation_error(
Expand Down
5 changes: 4 additions & 1 deletion src/api/promote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use super::import::{
};
use super::{
AppJson, AppPath, AppQuery, ErrorCode, Issue, RefusalDetail, access_error, deadline_exceeded,
error, key_name, ok_with_issues_total, validation_error,
error, key_name, ok_with_issues_total, overlong, validation_error,
};

#[derive(Debug, Deserialize)]
Expand Down Expand Up @@ -107,6 +107,9 @@ pub async fn promote_sources(
started_at,
);
}
if let Some(refusal) = overlong("sources", request.sources.len(), started_at) {
return refusal;
}
if request.into == name {
return error(
ErrorCode::InvalidArgument,
Expand Down
48 changes: 30 additions & 18 deletions src/benchmark/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,24 +517,32 @@ fn load_results(dir: &Path, manifest: &super::BenchManifest) -> Result<LoadedRes
Some("start") => {
starts.entry(document_id).or_insert(ts);
}
// The mirror image: a re-processed document
// logs a fresh `end`, and the LAST one is the
// state the cell finished in — keeping the
// first would freeze a superseded outcome
// while the per-line counters move on.
Some("end") => {
ends.entry(document_id).or_insert_with(|| DocumentEndRaw {
ts,
outcome: value
.get("outcome")
.and_then(Value::as_str)
.map(str::to_string),
associations: value.get("associations").and_then(Value::as_u64),
concepts: value.get("concepts").and_then(Value::as_u64),
labels: value.get("labels").and_then(Value::as_u64),
questions: value.get("questions").and_then(Value::as_u64),
duplicates: value.get("duplicates").and_then(Value::as_u64),
dropped: value.get("dropped").and_then(Value::as_u64),
batch_path: value
.get("batch_path")
.and_then(Value::as_str)
.map(str::to_string),
});
ends.insert(
document_id,
DocumentEndRaw {
ts,
outcome: value
.get("outcome")
.and_then(Value::as_str)
.map(str::to_string),
associations: value.get("associations").and_then(Value::as_u64),
concepts: value.get("concepts").and_then(Value::as_u64),
labels: value.get("labels").and_then(Value::as_u64),
questions: value.get("questions").and_then(Value::as_u64),
duplicates: value.get("duplicates").and_then(Value::as_u64),
dropped: value.get("dropped").and_then(Value::as_u64),
batch_path: value
.get("batch_path")
.and_then(Value::as_str)
.map(str::to_string),
},
);
}
_ => {}
}
Expand Down Expand Up @@ -852,7 +860,11 @@ fn attempt_distribution_metrics(attempts: &[&AttemptRow]) -> MetricsMap {

fn wall_seconds(doc: &DocRow) -> Option<f64> {
match (doc.start_ts, doc.end_ts) {
(Some(start), Some(end)) => Some(end - start),
// An end stamped before its start (a clock stepped mid-run, a
// resumed log's mismatched records) is a broken sample, not a
// negative duration — dropped like the degenerate inputs the
// per-attempt metrics below refuse.
(Some(start), Some(end)) if end >= start => Some(end - start),
_ => None,
}
}
Expand Down
65 changes: 65 additions & 0 deletions src/benchmark/compare/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,40 @@ fn wall_seconds_needs_both_start_and_end() {
assert_eq!(wall_seconds(&complete), Some(15.0));
}

#[test]
fn wall_seconds_drops_an_end_stamped_before_its_start() {
let backwards = DocRow {
cell_id: "m.run01".into(),
model_id: "m".into(),
run_index: 1,
document_id: "doc".into(),
start_ts: Some(10.0),
end_ts: Some(9.0),
outcome: None,
associations: None,
concepts: None,
labels: None,
questions: None,
duplicates: None,
dropped: None,
elapsed_seconds_sum: 0.0,
input_tokens_sum: None,
batch: None,
};
assert_eq!(
wall_seconds(&backwards),
None,
"a clock-stepped sample must not sink the distribution's min"
);
// A zero-length span is a real (if instant) measurement, not a
// broken one.
let instant = DocRow {
end_ts: Some(10.0),
..backwards
};
assert_eq!(wall_seconds(&instant), Some(0.0));
}

#[test]
fn document_outcome_rates_counts_interrupted_in_the_denominator_only() {
fn doc(outcome: Option<&str>) -> DocRow {
Expand Down Expand Up @@ -1218,6 +1252,37 @@ fn compute_measurements_over_a_synthetic_results_directory() {
let _ = fs::remove_dir_all(&dir);
}

#[test]
fn a_reprocessed_documents_second_end_record_supersedes_the_first() {
let dir = synthetic_results_dir("duplicate-end");
// A resumed cell that re-processed `brewery` logs a fresh `end`;
// the cell finished in THAT state, so the later record must win —
// mirroring `start`'s keep-the-earliest, not repeating it.
let runs_path = dir.join("runs/m.run01.jsonl");
let mut runs = fs::read_to_string(&runs_path).unwrap();
runs.push_str(
&serde_json::json!({
"kind": "document", "ts": 120.0, "cell_id": "m.run01",
"document_id": "brewery", "source": "corpus/brewery.md",
"document_sha256": "sha-brewery", "phase": "end", "outcome": "written",
"associations": 5, "concepts": 1, "labels": 0, "questions": 0,
"duplicates": 0, "dropped": 0, "batch_path": "cells/m/run01/brewery.jsonl",
})
.to_string(),
);
runs.push('\n');
fs::write(&runs_path, runs).unwrap();

let measurements = compute_measurements(&dir).expect("computes");
let brewery_run01 = &measurements.documents["m"]["brewery"]["run01"];
let MetricValue::Count(associations) = &brewery_run01["extraction.associations"] else {
panic!()
};
assert_eq!(associations.value(), Some(5.0));

let _ = fs::remove_dir_all(&dir);
}

#[test]
fn stability_metrics_with_a_single_run_are_the_defined_zero_shape() {
// synthetic_results_dir has exactly one run (m.run01) — every
Expand Down
19 changes: 19 additions & 0 deletions src/embedding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1439,6 +1439,17 @@ impl PassageAnnIndex {
/// index only ever costs recall, never correctness.
fn build(store: &PassageVectorStore, deadline: Deadline) -> Self {
let n = store.len();
// `centroid_count(0)` is still 1, so without this gate the
// seed row below would slice an empty `data` and panic —
// today's only caller sits behind the ANN threshold and can't
// pass 0 rows, but this function's contract shouldn't lean on
// that.
if n == 0 {
return Self {
centroids: Vec::new(),
lists: Vec::new(),
};
}
let dim = store.dim.max(1);
let target = Self::centroid_count(n);
let row = |i: usize| -> &[f32] { &store.data[i * dim..i * dim + dim] };
Expand Down Expand Up @@ -2438,6 +2449,14 @@ mod tests {
assert_eq!(PassageAnnIndex::centroid_count(PASSAGE_ANN_THRESHOLD), 100);
}

#[test]
fn passage_ann_index_build_on_an_empty_store_is_empty_not_a_panic() {
let store = synthetic_passage_store(0, 6);
let index = PassageAnnIndex::build(&store, Deadline::unbounded());
assert!(index.centroids.is_empty());
assert!(index.lists.is_empty());
}

#[test]
fn passage_ann_index_build_partitions_every_row_into_exactly_one_list() {
let rows = 500;
Expand Down
13 changes: 12 additions & 1 deletion src/evalset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ pub(crate) fn load_eval_file(path: &Path, mode: Extensions) -> Result<LoadedEval

let wire: WireCase = serde_json::from_value(value)
.map_err(|error| format!("{label}: line {number}: not a valid eval case: {error}"))?;
if wire.case_id.is_empty() {
if wire.case_id.trim().is_empty() {
return Err(format!("{label}: line {number}: case_id must not be empty"));
}
if wire.query.trim().is_empty() {
Expand Down Expand Up @@ -816,6 +816,17 @@ mod tests {
let _ = fs::remove_file(&path);
}

#[test]
fn a_whitespace_only_case_id_is_refused_like_an_empty_one() {
let path = write_temp(
"blank-case-id",
&format!("{HEADER}\n{{\"case_id\":\" \",\"query\":\"q\"}}\n"),
);
let error = load_eval_file(&path, Extensions::Interpret).unwrap_err();
assert!(error.contains("case_id must not be empty"), "{error}");
let _ = fs::remove_file(&path);
}

#[test]
fn an_empty_query_is_refused() {
let path = write_temp(
Expand Down
17 changes: 17 additions & 0 deletions src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,23 @@ mod tests {
);
}

/// A shipped-only report that no longer outruns the applied seq
/// clears the age — the same three-way move `note_replica_lane`
/// makes, so a lineage whose shipped seqs regress can never leave
/// a stale behind-since stamp on a lane that is in fact caught up.
#[test]
fn a_shipped_report_at_or_below_applied_clears_the_behind_age() {
let metrics = Metrics::default();
let key = ("sake".to_string(), "graph");
metrics.note_replica_lane("sake", "graph", 5, 5);
metrics.note_replica_shipped("sake", "graph", 7);
let stamped = metrics.replica_lag.lock()[&key].behind_since_epoch;
assert_ne!(stamped, 0, "a real gap starts the age");
metrics.note_replica_shipped("sake", "graph", 5);
let cleared = metrics.replica_lag.lock()[&key].behind_since_epoch;
assert_eq!(cleared, 0, "no gap, no age");
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
/// The in-flight counter: a ceiling refuses at capacity, zero means
/// count-only, release always returns the slot — and both series
/// render on /metrics.
Expand Down
13 changes: 10 additions & 3 deletions src/metrics/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,9 +464,16 @@ impl Metrics {
let mut lag = self.replica_lag.lock();
let entry = lag.entry((context.to_string(), lane)).or_default();
entry.shipped_seq = shipped_seq;
if entry.applied_seq < shipped_seq && entry.behind_since_epoch == 0 {
entry.behind_since_epoch = Self::unix_now();
}
// Same three-way move as `note_replica_lane`: today shipped
// seqs only grow within a lineage, so the caught-up arm can't
// fire here — but the reset must not silently depend on that.
entry.behind_since_epoch = if entry.applied_seq >= shipped_seq {
0
} else if entry.behind_since_epoch == 0 {
Self::unix_now()
} else {
entry.behind_since_epoch
};
}

/// Drops a vanished context's replica lag rows (both lanes).
Expand Down
Loading