Skip to content

fix: hoist _sessionAttentionState to module scope - #3826

Closed
happy5318 wants to merge 2 commits into
nesquena:masterfrom
happy5318:fix-session-attention-state-scope
Closed

happy5318 wants to merge 2 commits into
nesquena:masterfrom
happy5318:fix-session-attention-state-scope

Conversation

@happy5318

Copy link
Copy Markdown
Contributor

Problem

_sessionAttentionState was incorrectly defined inside renderSessionListFromCache() as a local function, but was called by _sidebarRowHasVisibleMessages() which is defined outside that scope.

This caused a runtime error:

Error: _sessionAttentionState is not defined

Root Cause

JavaScript function hoisting only works within the same function scope. The comment "available via function hoisting" was incorrect - the outer function cannot access inner function declarations.

Fix

Move _sessionAttentionState to module scope (top-level function) so all callers can access it.

Testing

  • Verified the function is now accessible from both _sidebarRowHasVisibleMessages and renderSessionListFromCache
  • Session list renders correctly after fix
  • No more console errors

The function was incorrectly defined inside renderSessionListFromCache()
as a local function, but was called by _sidebarRowHasVisibleMessages()
which is defined outside that scope. This caused a ReferenceError:
'_sessionAttentionState is not defined'.

JavaScript function hoisting only works within the same scope, so the
comment 'available via function hoisting' was incorrect.

Fix: Move _sessionAttentionState to module scope so all callers can access it.
@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a runtime ReferenceError caused by _sessionAttentionState being declared as a function inside renderSessionListFromCache() while being called from the module-scope function _sidebarRowHasVisibleMessages(). JavaScript function hoisting only lifts declarations within their enclosing function scope, so the outer caller could never resolve the name.

  • static/sessions.js: Moves _sessionAttentionState from inside renderSessionListFromCache() to module scope, making it reachable by both _sidebarRowHasVisibleMessages() and _renderOneSession() (which still calls it from inside renderSessionListFromCache()).
  • CHANGELOG.md: Adds a ### Fixed entry describing the crash and the corrective action.

Confidence Score: 5/5

Safe to merge — the change is a clean scope promotion of a single pure function with no logic changes.

The fix correctly resolves a real runtime crash by moving _sessionAttentionState to module scope. The function body is unchanged, all call sites remain valid, and the CHANGELOG is updated. No new risk is introduced.

No files require special attention.

Important Files Changed

Filename Overview
static/sessions.js Hoists _sessionAttentionState to module scope so _sidebarRowHasVisibleMessages can resolve it; _renderOneSession continues to work via normal scope-chain lookup. Change is a straight lift-and-shift with no logic modification.
CHANGELOG.md Adds a ### Fixed entry in [Unreleased] documenting the runtime crash and the fix, satisfying the AGENTS.md requirement to record user-visible behavior changes.

Sequence Diagram

sequenceDiagram
    participant SB as _sidebarRowHasVisibleMessages (module scope)
    participant SA as _sessionAttentionState (module scope — after fix)
    participant RL as renderSessionListFromCache
    participant RO as _renderOneSession (nested in RL)

    Note over SB,SA: Before fix: SA was only visible inside RL → ReferenceError
    Note over SB,SA: After fix: SA is at module scope → reachable everywhere

    SB->>SA: _sessionAttentionState(s)
    SA-->>SB: "{ kind, count, severity, label, title } | null"

    RL->>RO: calls _renderOneSession(s)
    RO->>SA: _sessionAttentionState(s)
    SA-->>RO: "{ kind, count, severity, label, title } | null"
Loading

Reviews (2): Last reviewed commit: "docs: add CHANGELOG entry for _sessionAt..." | Re-trigger Greptile

Comment thread static/sessions.js
Comment on lines +4097 to +4109
function _sessionAttentionState(s){
const attention=s&&s.attention&&typeof s.attention==='object'?s.attention:null;
if(!attention||!attention.kind||!Number.isFinite(Number(attention.count))||Number(attention.count)<=0)return null;
const kind=String(attention.kind)==='approval'?'approval':(String(attention.kind)==='clarify'?'clarify':'attention');
const count=Math.max(1,Number(attention.count)||1);
const labelKey=kind==='approval'?'session_attention_approval':(kind==='clarify'?'session_attention_clarify':'session_attention_generic');
const titleKey=kind==='approval'?'session_attention_approval_title':(kind==='clarify'?'session_attention_clarify_title':'session_attention_generic_title');
const fallback=kind==='approval'?(count===1?'Approval':`${count} approvals`):(kind==='clarify'?(count===1?'Question':`${count} questions`):(count===1?'Attention':`${count} items`));
const titleFallback=kind==='approval'?'Waiting for permission decision':(kind==='clarify'?'Waiting for your answer':'Waiting for user action');
const label=(typeof t==='function')?t(labelKey,count):fallback;
const title=(typeof t==='function')?t(titleKey,count):titleFallback;
return {kind,count,severity:String(attention.severity||''),label,title};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 CHANGELOG.md not updated for user-visible bug fix

Per AGENTS.md, "Update CHANGELOG.md for user-visible behavior, setup, workflow, or documentation changes that should be release-note ready." This fix resolves a runtime error (_sessionAttentionState is not defined) that broke session list rendering — it's a user-visible change that should be recorded in CHANGELOG.md before merging.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Closing as superseded — this is already fixed on master. _sessionAttentionState is defined at module scope in current static/sessions.js (single definition), and the surrounding code comment documents that the "ReferenceError: _sessionAttentionState is not defined" sidebar crash (#3696, regressed in #3672) was already hoisted to top level. Thanks @happy5318 — the diagnosis is exactly right, but the fix landed via the session-attention-badges work before this PR. Verified: grep shows one top-level definition on master, no nested copy remaining.

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