Skip to content

feat(temporal): refuse event, system, and available time as cutoff - #125

Merged
seonghobae merged 5 commits into
mainfrom
agent/cutoff-clock-identity
Aug 24, 2026
Merged

seonghobae merged 5 commits into
mainfrom
agent/cutoff-clock-identity

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Summary

  • ADR 0002 still required knowledge cutoff to stay distinct after available_clock (feat(temporal): refuse event and system time as availability #124).
  • Adds standalone cutoff_clock: event time, system time, and availability time cannot stand in for knowledge cutoff.
  • Recovered cutoff stamps match known truth at a higher computed rate than treating every stamp as availability.
  • Does not recreate available_clock, document_clocks, temporal_core, or membership_cutoff.

Claim boundary

  • Clock-identity gate only. No new migration number (#45 still owns 0007).
  • Not interval cutoff evaluation or a persistence change.

Test plan

  • RED: cutoff_clock failed with no library targets found
  • cargo test -p cutoff_clock --offline --lib --tests
  • cargo clippy -p cutoff_clock --all-targets --offline -- -D warnings
  • cargo llvm-cov -p cutoff_clock lines 73/73; nightly-2026-08-01 branches 6/6
  • docstring, workspace, and documentation gates
  • Exact-head CI and independent review

Add standalone cutoff_clock so knowledge cutoff stays distinct from
event time, system time, and availability time (ADR 0002). Recovered
cutoff stamps match known truth at a higher computed rate than treating
every stamp as availability. Does not allocate migration 0008 or
recreate available_clock, document_clocks, or membership_cutoff.
@coderabbitai

coderabbitai Bot commented Aug 16, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 44 minutes.

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: ceb658d3-ab5d-4a51-a646-bf422f867e4c

📥 Commits

Reviewing files that changed from the base of the PR and between fd051b7 and 476152b.

⛔ 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/cutoff_clock/Cargo.toml
  • crates/cutoff_clock/src/clock.rs
  • crates/cutoff_clock/src/error.rs
  • crates/cutoff_clock/src/lib.rs
  • crates/cutoff_clock/tests/crate_contract.rs
  • crates/cutoff_clock/tests/cutoff_clock_contract.rs
  • docs/TRACEABILITY.md
  • docs/adr/0002-six-clock-temporal-semantics.md
  • docs/adr/README.md
  • docs/research/cutoff-clock-identity.md
  • docs/research/standards-and-literature.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.

@seonghobae
seonghobae marked this pull request as ready for review August 20, 2026 18:36
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current head fbba5d6 is synchronized with origin/main. The quality contract was repaired at its root: tests/quality/test_check_docstrings.py now binds the crate count to check_workspace_contract.EXPECTED_CRATES instead of assuming ten crates after cutoff_clock was added.

Validation on this exact head: cargo fmt --all -- --check; cargo test -p cutoff_clock --offline (6 tests, all passed); full Python quality suite (89 tests, all passed); python scripts/check_workspace_contract.py; python scripts/validate_documentation.py; python scripts/check_docstrings.py; git diff --check.

No merge is requested until a qualifying independent approval and current required checks 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 fbba5d6:

  • Cutoff validation refuses event, system, and availability clocks as substitutes for the knowledge-cutoff clock, preserving historical eligibility semantics.
  • git diff --check, documentation validation, workspace contract, docstring contract, and cargo fmt --all -- --check passed locally.
  • Current hosted checks show 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 c225f35 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
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review completed for exact head c225f353f27647268d44a9e8156412a1b9de9fc2 against base c45be17a9dbce95ef81cee230e9d128abc7160ac.

  • Knowledge cutoff is kept distinct from event, system, and availability time; cutoff eligibility recovery is evaluated against known truth and invalid payloads fail closed.
  • The nominal KnowledgeCutoff clock remains a separate temporal type, preserving the availability-time cutoff contract.
  • Local pinned verification passed: cargo fmt --all -- --check; cargo test -p cutoff_clock --all-features; package clippy with -D warnings; workspace contract; documentation validation; Rust docstring contract; git diff --check.
  • All exact-head hosted checks are green, including Live PostgreSQL, Strix, coverage, SAST, dependency, Noema, and OpenCode.
  • No actionable current-head finding remains. No qualifying independent human approval is present; protected merge remains withheld. Decision: WAIT_AND_REMEDIATE.

@github-actions
github-actions Bot disabled auto-merge August 24, 2026 08:19
…entity

# Conflicts:
#	ARCHITECTURE.md
#	CHANGELOG.md
#	Cargo.lock
#	Cargo.toml
#	README.md
#	docs/TRACEABILITY.md
#	docs/adr/0002-six-clock-temporal-semantics.md
#	docs/adr/README.md
#	docs/research/standards-and-literature.md
#	docs/validation/temporal-event-foundation.md
#	scripts/check_workspace_contract.py
#	tests/quality/test_check_docstrings.py
@seonghobae
seonghobae merged commit 4136e6d 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 3 new potential issues.

Open in Devin Review

Comment on lines +70 to +74
for (truth_flag, decided_flag) in truth.iter().zip(decided) {
if truth_flag == decided_flag {
matches += 1;
}
}

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: Coverage of the non-matching branch relies on integration tests

The local unit tests in clock.rs only exercise eligibility_recovery_rate with matching flags (&[true],&[true]), so the truth_flag == decided_flag false branch is never taken there. The false branch is only covered by recovered_cutoff_stamps_match_known_truth_better_than_available_stand_in in cutoff_clock_contract.rs (collapsed_flags all false vs truth all true). This is fine because cargo llvm-cov -p aggregates lib and integration tests, but worth noting the 100% branch claim depends on the integration test being run.

Open in Devin Review

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

Comment on lines +1 to +17
[package]
name = "cutoff_clock"
description = "Knowledge cutoff cannot be replaced by event, system, or availability time."
version.workspace = true
edition.workspace = true
rust-version.workspace = true
license.workspace = true
authors.workspace = true
repository.workspace = true
homepage.workspace = true
readme.workspace = true
keywords.workspace = true
categories.workspace = true
publish = false

[lints]
workspace = true

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: cutoff_clock crate satisfies the workspace contract checks

I verified the new crate against check_workspace_contract.py: Cargo.toml sets publish = false, inherits all required workspace fields, and enables [lints] workspace = true; src/lib.rs contains #![forbid(unsafe_code)], #![deny(missing_docs)], and a //! crate doc; and tests/crate_contract.rs exists. EXPECTED_CRATES ordering (cutoff_clock last) matches both members and default-members in Cargo.toml. No contract violation found.

Open in Devin Review

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

Comment on lines +62 to +76
pub fn eligibility_recovery_rate(
truth: &[bool],
decided: &[bool],
) -> Result<f64, CutoffClockError> {
if truth.is_empty() || truth.len() != decided.len() {
return Err(CutoffClockError::InvalidCutoffPayload);
}
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: eligibility_recovery_rate handles empty and mismatched slices fail-closed

eligibility_recovery_rate (clock.rs) returns InvalidCutoffPayload when truth is empty or lengths differ, then divides matches by truth.len(), which is guaranteed non-zero at that point, so there is no division-by-zero risk. The zip iteration is correct given the equal-length guarantee. Logic is sound.

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