feat(analysis): bind topic activity/dormancy/reactivation to an analysis-run profile - #407
feat(analysis): bind topic activity/dormancy/reactivation to an analysis-run profile#407seonghobae wants to merge 1 commit into
Conversation
…sis-run profile Cutoff-safe topic_activity_v1 invokes topic_lineage dormancy/reactivation and remint refusal. Reactivation is not a new topic; not birth/split/merge and not a Bayesian sampler.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Author note (COMMENTED, not APPROVE): this slice binds topic_lineage dormancy/reactivation and remint refusal into topic_activity_v1. Reactivation is not a new topic. Not a Bayesian sampler and not birth/split/merge. Independent non-author APPROVE still required.
|
Warning Review limit reachedNext included review available in 30 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| pub struct TopicActivityInput { | ||
| identity: TopicIdentity, | ||
| transitions: Vec<TopicActivityTransition>, | ||
| proposed_reactivation_identity: TopicIdentity, | ||
| truth: Vec<TopicIdentity>, | ||
| decided: Vec<TopicIdentity>, | ||
| } |
There was a problem hiding this comment.
🔴 Activity history bypasses cutoff binding
TopicActivityInput carries neither snapshot nor availability metadata. Callers can label arbitrary transitions with the requested snapshot and cutoff, producing a falsely historical artifact.
Prompt for agents
Redesign TopicActivityInput and execute_topic_activity_run so each activity observation is immutably bound to its source snapshot and availability time. Validate that the input snapshot matches the request and exclude or reject every transition unavailable at the requested KnowledgeCutoff before applying the state sequence. Keep the validated cutoff-derived state in the digest-bound artifact and add tests for future-available transitions and cross-snapshot input.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if request.knowledge_cutoff != knowledge_cutoff.to_rfc3339() | ||
| || request.model_contract_version != TOPIC_ACTIVITY_MODEL_CONTRACT_VERSION | ||
| || request.output_profile != TOPIC_ACTIVITY_OUTPUT_PROFILE | ||
| { |
There was a problem hiding this comment.
🟡 Equivalent cutoffs reject valid runs
execute_topic_activity_run compares timestamp text instead of instants. Valid equivalent offsets fail with InvalidEvidence, although the API accepts them.
| if request.knowledge_cutoff != knowledge_cutoff.to_rfc3339() | |
| || request.model_contract_version != TOPIC_ACTIVITY_MODEL_CONTRACT_VERSION | |
| || request.output_profile != TOPIC_ACTIVITY_OUTPUT_PROFILE | |
| { | |
| if KnowledgeCutoff::parse_rfc3339(&request.knowledge_cutoff) | |
| .map_err(|_| AnalysisEngineError::InvalidEvidence)? | |
| != knowledge_cutoff | |
| || request.model_contract_version != TOPIC_ACTIVITY_MODEL_CONTRACT_VERSION | |
| || request.output_profile != TOPIC_ACTIVITY_OUTPUT_PROFILE | |
| { |
Was this helpful? React with 👍 or 👎 to provide feedback.
| TopicActivityTransition::Reactivate => record.reactivate()?, | ||
| }; | ||
| } | ||
| refuse_new_identity_on_reactivation(record.identity(), input.proposed_reactivation_identity())?; |
There was a problem hiding this comment.
🟡 Dormant runs require reactivation identity
execute_topic_activity_run enforces the proposed identity when no reactivation occurred. Active and dormant runs can fail over an irrelevant value.
Prompt for agents
Apply refuse_new_identity_on_reactivation only when the transition sequence contains Reactivate, or represent the proposed reactivation identity as optional and reject inconsistent presence. Add empty-sequence and dormant-only tests so both states can produce artifacts without supplying a fictional reactivation proposal.
Was this helpful? React with 👍 or 👎 to provide feedback.
| || !matches!(self.activity.as_str(), "active" | "dormant" | "reactivated") | ||
| || !self.identity_recovery_rate.is_finite() | ||
| || self.identity_recovery_rate < 0.0 | ||
| || self.identity_recovery_rate > 1.0 | ||
| || !self.reactivation_identity_preserved | ||
| || self.inference_status != TOPIC_ACTIVITY_INFERENCE_STATUS |
There was a problem hiding this comment.
🟡 Contradictory activity artifacts pass validation
TopicActivityArtifact::from_json accepts unreachable state/count combinations. A reactivated topic with zero transitions passes validation and can receive a valid digest.
Prompt for agents
Extend TopicActivityArtifact::validate to enforce state/count consistency from the executor's fixed active initial state: active requires zero transitions, dormant requires an odd positive count, and reactivated requires an even positive count. Add tampering tests for each contradictory combination.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub fn new( | ||
| identity: TopicIdentity, | ||
| transitions: Vec<TopicActivityTransition>, | ||
| proposed_reactivation_identity: TopicIdentity, | ||
| truth: Vec<TopicIdentity>, | ||
| decided: Vec<TopicIdentity>, | ||
| ) -> Self { |
There was a problem hiding this comment.
🔍 Activity input lacks resource bounds
TopicActivityInput::new accepts unlimited transitions and scoring identities. Add explicit count limits before exposing this profile through a service boundary.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ## Verification | ||
|
|
||
| The PR includes Rust unit and integration tests for dormancy-then-reactivation | ||
| identity preservation, remint refusal, illegal transitions, minted-replacement | ||
| recovery rates, snapshot/profile/cutoff mismatch, and artifact tampering. Run: | ||
|
|
||
| ```text | ||
| cargo fmt --all -- --check | ||
| cargo test -p analysis_engine | ||
| cargo clippy -p analysis_engine --all-targets -- -D warnings | ||
| python3 scripts/validate_documentation.py | ||
| ``` |
There was a problem hiding this comment.
🔍 Coverage gate evidence is missing
The verification list omits the mandatory 100% line-and-branch coverage gate. Confirm exact-head coverage for every new validation and execution outcome.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
GAP-004 leftover / issue #167 / ADR 0051. Bind existing
topic_lineageactivity/dormancy/reactivation and remint refusal to a cutoff-safe analysis-run profile so an operator can request a digest-bound identity artifact.topic_activity_v1/ schematepp.topic_activity.v1TopicLineageRecorddormancy/reactivation,refuse_new_identity_on_reactivation, andidentity_recovery_ratereactivation_is_not_new_topic_not_birth_split_mergeNot a Bayesian sampler. Not topic birth/split/merge. Not
trsl_topic_lineage_v1sequence edges. Not implemented-main.Verification
cargo test -p analysis_enginecargo clippy -p analysis_engine --all-targets -- -D warningspython3 scripts/validate_documentation.pyMerge gate
Two independent current-head APPROVEs required. Author COMMENTED is not APPROVE. Do not self-approve.