feat: add relationship-aware NPC society scheduling#108
Conversation
|
Codex (@codex) review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3006301c4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| id: "society-decision-proactive-" + sanitizeNakamaIdentifier(profile.actor_id, "npc") + "-" + (index + 1), | ||
| actor_id: profile.actor_id, | ||
| display_name: profile.display_name, | ||
| trigger_event_id: "proactive-" + sanitizeNakamaIdentifier(playerActorId, "nearby-player"), |
There was a problem hiding this comment.
Generate unique proactive trigger_event_id per schedule tick
Using a fixed trigger_event_id of proactive-<player> for every proactive decision makes all future proactive schedules for that player map to the same conversation session key (createOrUpdateConversationSession derives session_id from trigger_event_id when no explicit session id is passed). After that session reaches max_turns and becomes closed, later proactive ticks keep reusing the closed session, so new proactive exchanges inherit stale transcript/objective state instead of starting fresh.
Useful? React with 👍 / 👎.
| function shouldBuildProactiveSocietyQueue(request: any): boolean { | ||
| return trimString(request.mode) === "proactive" || | ||
| trimString(request.nearby_player_actor_id || request.player_actor_id) !== ""; |
There was a problem hiding this comment.
Respect trigger_event_id when deciding proactive fallback
The queue now appends proactive candidates whenever a player actor id is present, even in reactive requests that explicitly set trigger_event_id. This bypasses the event filter and can return decisions unrelated to the requested event replay, which breaks deterministic processing for callers that expect trigger_event_id to strictly scope scheduling.
Useful? React with 👍 / 👎.
| agent_context: compactAgentDecisionContext(context), | ||
| knowledge_packs: buildDecisionKnowledgePacks(nk, context, world), | ||
| conversation_session: buildDecisionConversationSession(nk, ownerId, world), | ||
| anti_repeat: buildDecisionAntiRepeatContext(nk, ownerId, world), |
There was a problem hiding this comment.
Include anti_repeat in DOS.AI token budget estimation
The prompt now sends an extra anti_repeat block, but the budget gate still uses estimateDosAiDecisionTokens without accounting for this added payload. In environments with tighter DOS_AI_DECISION_DAILY_TOKEN_BUDGET and larger recent speech/session history, requests can be admitted after the intended cap should have been reached, weakening cost and rate-limit controls.
Useful? React with 👍 / 👎.
Summary
Local review
Tests