Skip to content

feat(event): refuse subevents that escape the parent interval - #118

Merged
seonghobae merged 4 commits into
mainfrom
agent/subevent-containment
Aug 24, 2026
Merged

feat(event): refuse subevents that escape the parent interval#118
seonghobae merged 4 commits into
mainfrom
agent/subevent-containment

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Summary

  • ADR 0003 still required subevents to stay inside the parent event instance.
  • Adds standalone subevent_containment: a half-open child interval that starts before or ends after the parent cannot attach.
  • Recovered containment flags match known truth at a higher computed rate than accepting every child.
  • Does not persist subevents or recreate event_core mention/instance separation.

Claim boundary

  • Interval containment only. No new migration number (#45 still owns 0007).
  • Not Allen composition, mention promotion, or a full event-ontology store.

Test plan

  • RED: containment_contract failed with E0432
  • cargo test -p subevent_containment --offline --lib --tests
  • cargo clippy -p subevent_containment --all-targets --offline -- -D warnings
  • cargo llvm-cov -p subevent_containment lines 20/20; nightly-2026-08-01 branches 12/12
  • docstring, workspace, and documentation gates
  • Exact-head CI and independent review

A half-open subevent window must lie inside the parent event-time
interval (ADR 0003).
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 3 seconds.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 343e1d53-e2ee-4fd6-a55e-1846c92737f9

📥 Commits

Reviewing files that changed from the base of the PR and between d8e7138 and 8e15967.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (17)
  • ARCHITECTURE.md
  • CHANGELOG.md
  • Cargo.toml
  • README.md
  • crates/subevent_containment/Cargo.toml
  • crates/subevent_containment/src/error.rs
  • crates/subevent_containment/src/interval.rs
  • crates/subevent_containment/src/lib.rs
  • crates/subevent_containment/tests/containment_contract.rs
  • crates/subevent_containment/tests/crate_contract.rs
  • docs/TRACEABILITY.md
  • docs/adr/0003-relational-event-multiple-membership.md
  • docs/adr/README.md
  • docs/research/standards-and-literature.md
  • docs/research/subevent-containment.md
  • docs/validation/temporal-event-foundation.md
  • scripts/check_workspace_contract.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

# Conflicts:
#	CHANGELOG.md
#	docs/research/standards-and-literature.md
#	docs/validation/temporal-event-foundation.md
@seonghobae
seonghobae marked this pull request as ready for review August 20, 2026 18:55
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current HEAD 97c6363a37aa579abaf3a5622e40d0a2275c7797 includes current main, the half-open parent/child interval containment gate, and the dynamic workspace crate-count quality fix. Verified at this HEAD: cargo fmt --all -- --check; cargo test -p subevent_containment --offline (8 tests including escaped-boundary and known-truth recovery contracts); Python quality suite (89 tests); workspace, documentation, Rust docstring, and diff checks. Please provide the required independent approval; merge remains gated until current-head required checks and approval are present.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review refresh for 97c6363:

  • Subevent intervals are required to remain within their parent event interval, preserving event-time containment.
  • git diff --check, documentation validation, workspace contract, docstring contract, and cargo fmt --all -- --check passed locally.
  • Current hosted checks are pass=15, pending=1, fail=0; merge remains withheld until the pending exact-head required check completes and qualifying approvals exist.
  • No new actionable source defect was found at this exact head; no approval is being self-issued.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Rebased current head b8f4582 onto origin/main. The changelog conflict retains both feature and current-main entries; inherited documentation trailing whitespace was removed. Local merge-tree, git diff --cached --check, and cargo fmt --all -- --check pass. Exact-head hosted checks and required independent approvals remain required before protected merge.

@seonghobae
seonghobae enabled auto-merge (squash) August 24, 2026 00:57
…nment

# Conflicts:
#	ARCHITECTURE.md
#	CHANGELOG.md
#	Cargo.toml
#	README.md
#	docs/TRACEABILITY.md
#	docs/adr/0003-relational-event-multiple-membership.md
#	docs/adr/README.md
#	docs/validation/temporal-event-foundation.md
#	scripts/check_workspace_contract.py
#	tests/quality/test_check_docstrings.py
@github-actions
github-actions Bot disabled auto-merge August 24, 2026 08:18
@seonghobae
seonghobae merged commit ab78015 into main Aug 24, 2026
19 of 22 checks passed

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 5 new potential issues.

Open in Devin Review


International Organization for Standardization and International Electrotechnical Commission. (2011). *Information technology—Security techniques—Privacy framework* (ISO/IEC Standard No. 29100:2011). Data minimization informs `provider_receipt`; it is not a certification claim.

Allen, J. F. (1983). Maintaining knowledge about temporal intervals. *Communications of the ACM, 26*(11), 832–843. https://doi.org/10.1145/182.358434. The `during` relation informs `subevent_containment`; the crate does not implement Allen composition.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Temporal reference filed under privacy section

The Allen (1983) citation for subevent_containment at standards-and-literature.md is placed under ## Privacy and operational log/source separation, not the ## Time, events, and topic detection and tracking section (line 70) that already lists the same paper.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +52 to +57
pub fn interval_contains(
parent: EventInterval,
child: EventInterval,
) -> Result<bool, SubeventContainmentError> {
Ok(child.start_seconds >= parent.start_seconds && child.end_seconds <= parent.end_seconds)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: interval_contains documented as infallible yet returns Result

interval_contains at interval.rs always returns Ok(..) and its # Errors docstring states it is infallible; the Result wrapper and #[allow(clippy::unnecessary_wraps)] exist only to keep the public surface explicit. refuse_escaped_subevent propagates the never-taken ?. This is intentional per the docs but worth noting: consumers must still handle an error arm that can never occur, which slightly complicates the API contract.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +52 to +57
pub fn interval_contains(
parent: EventInterval,
child: EventInterval,
) -> Result<bool, SubeventContainmentError> {
Ok(child.start_seconds >= parent.start_seconds && child.end_seconds <= parent.end_seconds)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Containment logic is correct for half-open intervals

interval_contains at interval.rs correctly implements half-open [start, end) containment: a child is contained iff child.start >= parent.start && child.end <= parent.end. EventInterval::new rejects zero-length/inverted intervals (end <= start), so the recovery and refusal paths cannot receive degenerate intervals. No correctness issue found.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +81 to +95
pub fn containment_recovery_rate(
truth: &[bool],
decided: &[bool],
) -> Result<f64, SubeventContainmentError> {
if truth.is_empty() || truth.len() != decided.len() {
return Err(SubeventContainmentError::InvalidIntervalPayload);
}
let mut matches = 0_u32;
for (truth_flag, decided_flag) in truth.iter().zip(decided) {
if truth_flag == decided_flag {
matches += 1;
}
}
Ok(f64::from(matches) / truth.len() as f64)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Recovery-rate empty/length checks match documented contract

containment_recovery_rate at interval.rs returns InvalidIntervalPayload when truth.is_empty() or lengths differ. The docstring says "when either slice is empty or the lengths differ"; the empty-decided/non-empty-truth case is caught by the length check, and empty truth is caught directly, so the two conditions together cover all empty cases. Behavior is consistent with the contract.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

"tepp_simulation",
"validation_core",
"tepp_api",
"subevent_containment",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Workspace member ordering stays in sync across manifest, contract, and README

The new subevent_containment crate is appended consistently to Cargo.toml members and default-members, to EXPECTED_CRATES in check_workspace_contract.py (which enforces exact ordered equality of members/default-members), and the README crate list. The docstring test was updated to assert len(crate_roots) == len(contract.EXPECTED_CRATES) rather than a hardcoded 10, so it will not drift. The mechanical transformation appears complete and consistent.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant