fix: using cookies for session_id

[BayerC]

No description provided.

[sourcery-ai]

Hey - I've left some high level feedback:

• Switching from query params to the ajs_anonymous_id cookie changes the session ID source but you no longer persist the generated UUID back to cookies, so consider explicitly setting a cookie when you create a new session ID to keep behavior consistent across requests.
• The hard-coded ajs_anonymous_id cookie name couples this logic to a specific analytics cookie; consider making the cookie name configurable or clearly tied to your own session cookie instead of a third-party naming convention.
• If st.context or st.context.cookies can be absent in some environments, you may want to defensively handle those cases to avoid attribute errors when initializing SessionState.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Switching from query params to the `ajs_anonymous_id` cookie changes the session ID source but you no longer persist the generated UUID back to cookies, so consider explicitly setting a cookie when you create a new session ID to keep behavior consistent across requests.
- The hard-coded `ajs_anonymous_id` cookie name couples this logic to a specific analytics cookie; consider making the cookie name configurable or clearly tied to your own session cookie instead of a third-party naming convention.
- If `st.context` or `st.context.cookies` can be absent in some environments, you may want to defensively handle those cases to avoid attribute errors when initializing `SessionState`.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (7f7ea7e) to head (598ba76).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #166   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          529       526    -3     
=========================================
- Hits           529       526    -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cbachhuber]

Should we even do this or? I'd prefer to error out and thus know if the cookie thing doesn't work

[BayerC]

yeah good point, done

[BayerC]

@sourcery-ai review

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• Accessing st.context.cookies["ajs_anonymous_id"] without a default or existence check may raise errors when the cookie is missing; consider using .get(...) with a fallback or preserving the UUID generation as a backup.
• Previously, the session_id was also pushed into st.query_params; if any other part of the app still relies on session_id being present in the query string, this change could break that behavior—consider verifying and updating those call sites or keeping the sync if needed.
• Hard-coding the analytics cookie key "ajs_anonymous_id" inside SessionState tightly couples session ID handling to a specific cookie name; consider centralizing this key in a configuration or helper to make future changes easier.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Accessing `st.context.cookies["ajs_anonymous_id"]` without a default or existence check may raise errors when the cookie is missing; consider using `.get(...)` with a fallback or preserving the UUID generation as a backup.
- Previously, the `session_id` was also pushed into `st.query_params`; if any other part of the app still relies on `session_id` being present in the query string, this change could break that behavior—consider verifying and updating those call sites or keeping the sync if needed.
- Hard-coding the analytics cookie key `"ajs_anonymous_id"` inside `SessionState` tightly couples session ID handling to a specific cookie name; consider centralizing this key in a configuration or helper to make future changes easier.

## Individual Comments

### Comment 1
<location path="src/open_cups/session_state.py" line_range="13" />
<code_context>
-            existing = st.query_params.get("session_id")
-            st.session_state.session_id = existing or str(uuid.uuid4())
-        st.query_params["session_id"] = st.session_state.session_id
+            st.session_state.session_id = st.context.cookies["ajs_anonymous_id"]

     @property
</code_context>
<issue_to_address>
**issue:** Accessing the cookie with a direct index can raise a KeyError when the cookie is missing.

The previous implementation generated a new UUID when `session_id` was missing, but the new code assumes `"ajs_anonymous_id"` always exists and will raise if it doesn’t (e.g., first visit, cookies disabled, different tracker setup). Please use `.get("ajs_anonymous_id")` with a sensible fallback (such as generating a UUID) or otherwise handle the missing-cookie case explicitly.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue: Accessing the cookie with a direct index can raise a KeyError when the cookie is missing.

The previous implementation generated a new UUID when session_id was missing, but the new code assumes "ajs_anonymous_id" always exists and will raise if it doesn’t (e.g., first visit, cookies disabled, different tracker setup). Please use .get("ajs_anonymous_id") with a sensible fallback (such as generating a UUID) or otherwise handle the missing-cookie case explicitly.

[cbachhuber]

Thanks!