Skip to content

docs: add canonical session resolution contract - #2922

Merged
1 commit merged into
nesquena:masterfrom
ai-ag2026:docs/canonical-session-resolution
May 25, 2026
Merged

1 commit merged into
nesquena:masterfrom
ai-ag2026:docs/canonical-session-resolution

Conversation

@ai-ag2026

Copy link
Copy Markdown
Contributor

Thinking Path

Recent compression/session-routing bugs keep surfacing the same state-boundary problem: URL routes, query parameters, localStorage, sidebar collapse, and compression lineage can all identify different rows for one logical conversation. The data is usually still present, but the browser can reopen a stale parent snapshot or a non-canonical row and make the session look lost.

Rather than turning that into another broad implementation PR, this adds a narrow public contract so future fixes can point to one review target.

What Changed

  • Added docs/rfcs/canonical-session-resolution.md.
  • Linked the RFC from docs/rfcs/README.md and docs/CONTRACTS.md.
  • Added a small pytest guard that keeps the RFC indexed and verifies the contract names the required entrypoints and outputs.
  • Added an Unreleased changelog entry.

Why It Matters

This gives future session-routing fixes a shared vocabulary:

  • requested session ID
  • canonical_visible_session_id
  • pre_compression_snapshot
  • continuation_session_id
  • URL/query/localStorage/sidebar/direct-open entrypoints

It complements #2361 and #1925 without implementing a new backend surface yet.

Verification

  • RED: uv run --with pytest python -m pytest tests/test_canonical_session_resolution_rfc.py -q -o addopts='' failed before the RFC existed.
  • GREEN: uv run --with pytest python -m pytest tests/test_canonical_session_resolution_rfc.py -q -o addopts='' -> 2 passed.
  • Public hygiene scan: 0 added private/local marker hits, 0 secret-like added-line hits.

Risks / Follow-ups

Model Used

OpenAI Codex / GPT-5.5 via Hermes Agent, with terminal/file tools and TDD workflow.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Read the RFC at docs/rfcs/canonical-session-resolution.md plus the indexing test. The contract is well-scoped and the test guards index linkage + required terminology.

Code reference

The two test cases pin both the linkage and the contract surface:

def test_canonical_session_resolution_rfc_is_indexed():
    ...
    assert "canonical-session-resolution.md" in rfc_index
    assert rel in contracts


def test_canonical_session_resolution_contract_names_entrypoints_and_outputs():
    text = RFC.read_text(encoding="utf-8")
    required_terms = [
        "URL route", "query parameter", "localStorage", "sidebar",
        "pre_compression_snapshot", "canonical_visible_session_id",
        "continuation_session_id", "parent_session_id",
        "direct session open", "browser boot restore",
    ]
    missing = [term for term in required_terms if term not in text]
    assert missing == []

The terminology table at the top of the RFC matches existing field names already in use across the codebase (verified pre_compression_snapshot, parent_session_id, _lineage_root_id, _lineage_tip_id, _compression_segment_count in static/sessions.js:2640 and the freshly-merged work). That alignment is what makes the contract reviewable rather than aspirational.

One observation about scope

Resolution Rule 2 ("snapshot parents defer to visible continuation tips") is exactly what #2913 (fix(chat): keep compression tip selected in sidebar) just landed in static/sessions.js:2647-2678:

function _resolveSessionIdFromSidebarLineage(sid){
  ...
  // candidates sorted by segment count, then non-snapshot preference, then timestamp
  candidates.sort((a,b)=>{
    ...
    const bSnapshot=!!(b&&b.pre_compression_snapshot);
    const aSnapshot=!!(a&&a.pre_compression_snapshot);
    if(bSnapshot!==aSnapshot) return aSnapshot-bSnapshot;
    return _sessionTimestampMs(b)-_sessionTimestampMs(a);
  });

And Rule 5 (?session= / ?session_id= / /session/ share the same resolver) is what #2912 just landed in static/sessions.js:1556:

return qs.get('session')||qs.get('session_id')||null;

So the RFC is documenting a contract whose implementations are arriving alongside it. That's a reasonable order — the RFC gives the next round of fixes a shared vocabulary instead of each PR reinventing the rule set. The list at the bottom of the doc (Existing issue context: #2361, #1925, #2383, #2489, #2782, #2912, #2913) tracks that history well.

Suggestion

Two narrow things worth adding before merge, but neither is a blocker:

  1. The "Review Checklist" section is good prose but isn't anchored in code. Consider adding a short pointer for each checklist item to the helper or call site reviewers should grep — e.g. for "Does localStorage go through the same canonicalization path as URL restore?" mention _sessionIdFromLocation at static/sessions.js:1556 and the boot-restore caller. The current _resolveSessionIdFromSidebarLineage at :2647 would similarly anchor the "Can a pre_compression_snapshot become the default active chat" item. That makes the checklist actionable without re-reading the RFC every time.

  2. Rule 7 ("404 self-heal is separate from lineage resolution") references GET /api/session returns 200 for missing sessions but POST 404s — UI stuck with no self-heal path #2782. Worth a one-line example in the rule itself: "if the requested ID is missing entirely from /api/sessions, fall through to the stale-route recovery path; if it's present but pre_compression_snapshot, canonicalize to the continuation." Right now the rule reads more like a non-goal than a positive rule.

Diagnosis

Docs-only + a guard test. No runtime behavior change. The hygiene scan noted in the PR description (0 added private/local marker hits, 0 secret-like hits) confirms the doc doesn't leak machine paths. Approve as-is or take the two checklist anchor suggestions if you want them in this PR — either is fine. The contract is going to be referenced by the next several session-routing PRs, so landing it as-is is reasonable.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Merged in Release DG / v0.51.135 (stage-batch17, batch with PRs #2906 #2912 #2917 #2919 #2921 #2922 #2927 #2936 #2940).

Thanks @ai-ag2026! 🚢

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.

2 participants