Skip to content

feat(api): retrieve accepted temporal-context identities by id - #451

Closed
seonghobae wants to merge 1 commit into
mainfrom
feat/temporal-context-retrieval-get-gap-003a
Closed

feat(api): retrieve accepted temporal-context identities by id#451
seonghobae wants to merge 1 commit into
mainfrom
feat/temporal-context-retrieval-get-gap-003a

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Folded into #452

Closed as superseded_by_fold, not discarded. #452's head contains this PR as its direct ancestor and has been retargeted to protected main, so the temporal-context retrieval GET implementation/tests and this review history remain intact while queue WIP is reduced. Sibling stored-request vehicle #464 also retains this ancestry.

Canonical retrieval landing vehicle: #452 (feat(api): consolidate temporal-context retrieval GET and CLI).

Do not reopen unless a surviving folded head demonstrably loses unique behavior or evidence.

Publish GET /v1/temporal-context/{idempotency_key} so operators fetch one
metric-free LineageWeave identity without POST replay. Event labels and
actor lists never appear. Collection GET stays closed. Naruon is refused.
NaruonLiveService stays POST-only. ADR 0083.
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 30 minutes.

Check out review usage here.

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: Team

Run ID: af3e8fff-2b88-47a5-9bf8-1b0b5d26793c

📥 Commits

Reviewing files that changed from the base of the PR and between 1bc02f5 and 7b117e8.

📒 Files selected for processing (11)
  • CHANGELOG.d/temporal-context-retrieval-get.md
  • DOCUMENTATION.md
  • crates/tepp_api/src/analysis_run_live.rs
  • crates/tepp_api/src/lib.rs
  • crates/tepp_api/src/temporal_context_retrieval_http.rs
  • crates/tepp_api/tests/temporal_context_retrieval_http_contract.rs
  • docs/API_CONTRACT.md
  • docs/TRACEABILITY.md
  • docs/adr/0083-temporal-context-retrieval-get.md
  • docs/adr/README.md
  • docs/research/temporal-context-retrieval-get.md

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.

@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 7 potential issues.

Devin Review

Comment on lines +192 to +195
if let Some(stored) = self.accepted_temporal_contexts.get(&replay_key) {
if stored.knowledge_cutoff != item.knowledge_cutoff {
return Err(ApiError::InvalidWirePayload);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Conflicting retries share one identity

When a key is reused with different events but the same cutoff, accept_temporal_context treats both submissions as matching. The retry returns different context under one identity instead of failing closed.

Prompt for agents
The temporal-context replay registry in crates/tepp_api/src/analysis_run_live.rs stores only TemporalContextRetrieved and compares only knowledge_cutoff. Store enough validated request state to compare the complete TemporalContextRequest on a repeated consumer/idempotency key. Return the original result for an exact replay and reject any changed request, including changes to events or subject_post_id.
Devin Review

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

Comment on lines +196 to +197
} else {
self.accepted_temporal_contexts.insert(replay_key, item);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Failed submissions become retrievable

When response serialization fails, accepted_temporal_contexts.insert has already registered the identity. The failed submission then becomes retrievable as accepted.

Prompt for agents
In accept_temporal_context, complete temporal-context construction and response serialization before mutating accepted_temporal_contexts. Only commit the identity after every fallible response step succeeds, while preserving exact replay and conflict behavior.
Devin Review

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

Comment on lines +187 to +191
if payload.contains("tepp.scientific_acceptance.v1")
|| payload.contains("event_label")
|| payload.contains("actor_references")
|| payload.contains("timeline_events")
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Opaque keys become unreadable

Valid opaque keys can contain reserved-looking text such as event_label. to_json rejects those stored identities, so their POST succeeds but every GET returns 400.

Suggested change
if payload.contains("tepp.scientific_acceptance.v1")
|| payload.contains("event_label")
|| payload.contains("actor_references")
|| payload.contains("timeline_events")
{
let value: serde_json::Value =
serde_json::from_str(payload).map_err(|_| ApiError::InvalidWirePayload)?;
Devin Review

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

TEMPORAL_CONTEXT_RETRIEVAL_CONTRACT_VERSION,
)?;
validate_temporal_context_registry_identity(&self.idempotency_key)?;
require_nonempty(&self.knowledge_cutoff)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Malformed cutoffs pass validation

TemporalContextRetrieved::new accepts any nonempty cutoff instead of an RFC 3339 timestamp. Consumers can accept malformed retrieval payloads as validated identities.

Prompt for agents
Validate TemporalContextRetrieved.knowledge_cutoff with the same RFC 3339 knowledge-cutoff domain parser used by TemporalContextRequest. Apply this validation in the shared validate method so both construction and JSON parsing reject malformed timestamps.
Devin Review

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

Comment on lines +185 to +190
if let Some(idempotency_key) = headers.get("idempotency-key") {
let item = TemporalContextRetrieved::new(
idempotency_key.clone(),
context_request.knowledge_cutoff.clone(),
TEMPORAL_CONTEXT_RETRIEVAL_INFERENCE_STATUS,
)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Canonical POST builder cannot mint identities

lineageweave_temporal_context_exchange cannot add an idempotency key. Its callers cannot use the new retrieval flow without manually modifying the exchange.

Devin Review

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

Comment on lines +31 to +48
const FORBIDDEN_RETRIEVAL_KEYS: [&str; 16] = [
"rmse",
"rmse_standard_error",
"mean_bias",
"bias_standard_error",
"interval_coverage",
"coverage_wilson_lower",
"coverage_wilson_upper",
"temporal_order_accuracy",
"se_gate_accepted",
"se_gate_k",
"scientific_acceptance",
"report",
"terminal_result",
"evidence_text",
"findings",
"causal_score",
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Redundant metric filter can drift

deny_unknown_fields already excludes every metric field. The separate recursive denylist duplicates that contract and can diverge as fields evolve.

Devin Review

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

Comment on lines +151 to +153
if method == "GET" {
return self.get_temporal_context(path, &headers, body);
}

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: GET routing stays endpoint-scoped

temporal_context_retrieval_path_id admits only one temporal-context child segment. Collection, extra-segment, and unrelated GET routes remain closed.

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