-
Notifications
You must be signed in to change notification settings - Fork 0
feat(api): publish completed analysis-run result contract #157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a3c57a9
003dae3
0cb693f
cffbf4e
24f00c0
30918e5
020c353
f1c94f7
fb783f5
0cb9ff6
1fa8e9e
63a419e
7625dc6
6852e9d
95add88
e3770a6
910a54e
efd5386
48643e5
8e96984
a9a49d3
ef45763
3b07002
9238c3a
e06e504
12ada13
cce90a1
7402745
3400874
810e3a9
88f10c4
e22dcb1
41a7218
d245a9d
7521a15
f8de47e
e65023f
5d1492c
959108d
f31d822
f204ec9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,7 @@ members = [ | |
| "crates/compute_backend", | ||
| "crates/episode_membership", | ||
| "crates/membership_target", | ||
| "crates/analysis_engine", | ||
| "crates/topic_measurement", | ||
| "crates/analysis_engine", | ||
| "crates/psychometric_core", | ||
|
Comment on lines
+57
to
60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 analysis_engine listed twice in workspace members
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
@@ -112,6 +113,7 @@ default-members = [ | |
| "crates/compute_backend", | ||
| "crates/episode_membership", | ||
| "crates/membership_target", | ||
| "crates/analysis_engine", | ||
| "crates/topic_measurement", | ||
| "crates/analysis_engine", | ||
| "crates/psychometric_core", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| //! was unavailable at the requested knowledge cutoff, counts multiple-membership | ||
| //! assignments without collapsing them, and emits a digest-bound terminal result | ||
| //! through [`tepp_api`]. It deliberately does not claim latent-variable or topic | ||
| //! estimation authority; those estimators remain separate scientific crates. | ||
| //! estimation authority; it invokes estimators through their scientific crate | ||
| //! contracts and preserves their artifact meaning. | ||
|
|
||
|
|
@@ -303,6 +304,14 @@ pub fn execute_analysis_run( | |
| }); | ||
| } | ||
|
|
||
| // The corpus bound makes this conversion strictly smaller than | ||
| // `u64::MAX`; the fold still fails closed through checked arithmetic so a | ||
| // future bound change cannot wrap membership totals silently. | ||
| let eligible_evidence_count = eligible.len() as u64; | ||
| let eligible_membership_count = eligible.iter().try_fold(0_u64, |sum, unit| { | ||
| sum.checked_add(u64::from(unit.membership_count)) | ||
| .ok_or(AnalysisEngineError::ArithmeticOverflow) | ||
| })?; | ||
| // The corpus bound makes this conversion and sum strictly smaller than | ||
| // `u64::MAX`: 100,000 * u32::MAX is below the 64-bit range. | ||
| let eligible_evidence_count = eligible.len() as u64; | ||
|
Comment on lines
+307
to
317
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Overflow-checked membership total silently discarded The eligible membership total is first summed with checked arithmetic that fails closed on overflow, then immediately re-summed with an unchecked (Refers to this code) Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
@@ -378,6 +387,7 @@ mod tests { | |
| use super::{ | ||
| ANALYSIS_ARTIFACT_SCHEMA_VERSION, ANALYSIS_STATISTIC_COUNT, AnalysisCorpus, | ||
| AnalysisEngineError, AnalysisEvidenceUnit, MAX_ANALYSIS_IDENTIFIER_BYTES, | ||
| MAX_EVIDENCE_UNITS, execute_analysis_run, | ||
| MAX_EVIDENCE_UNITS, TopicMeasurementError, execute_analysis_run, | ||
| }; | ||
|
Comment on lines
387
to
392
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Test module has duplicate imports and truncated calls The test module imports Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| use temporal_core::{AvailableTime, EventTime}; | ||
|
|
@@ -529,7 +539,6 @@ mod tests { | |
| vec![unit( | ||
| "evidence-1", | ||
| "2026-07-01T00:00:00Z", | ||
| "2026-07-01T00:00:00Z", | ||
| 1, | ||
| )], | ||
| ) | ||
|
|
@@ -545,7 +554,6 @@ mod tests { | |
| vec![unit( | ||
| "evidence-1", | ||
| "2026-07-01T00:00:00Z", | ||
| "2026-07-01T00:00:00Z", | ||
| 1, | ||
| )], | ||
| ) | ||
|
|
@@ -586,7 +594,6 @@ mod tests { | |
| let evidence = unit( | ||
| "evidence-accessor", | ||
| "2026-07-01T00:00:00Z", | ||
| "2026-07-01T00:00:00Z", | ||
| 4, | ||
| ); | ||
| assert_eq!(evidence.evidence_id(), "evidence-accessor"); | ||
|
|
@@ -661,7 +668,6 @@ mod tests { | |
| vec![unit( | ||
| "evidence-1", | ||
| "2026-07-01T00:00:00Z", | ||
| "2026-07-01T00:00:00Z", | ||
| 1, | ||
| )], | ||
| ) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Duplicate, conflicting crate rows in topology table
The added rows for
validation_core,tepp_api, andlocation_membershipduplicate rows that already exist in the same table further down.validation_coreandtepp_apinow appear twice with different descriptions, so the table states two conflicting responsibilities for each.Was this helpful? React with 👍 or 👎 to provide feedback.