fix(session): skip LFS stubs in detect loop, re-summarize stub artifacts#428
Merged
rsnodgrass merged 3 commits intomainfrom Apr 4, 2026
Merged
fix(session): skip LFS stubs in detect loop, re-summarize stub artifacts#428rsnodgrass merged 3 commits intomainfrom
rsnodgrass merged 3 commits intomainfrom
Conversation
Co-Authored-By: SageOx <ox@sageox.ai> SageOx-Session: https://sageox.ai/repo/repo_019c5812-01e9-7b7d-b5b1-321c471c9777/sessions/2026-04-04T05-12-ryan-Oxm41G/view
…cts, strengthen code search prime guidance Co-Authored-By: SageOx <ox@sageox.ai> SageOx-Session: https://sageox.ai/repo/repo_019c5812-01e9-7b7d-b5b1-321c471c9777/sessions/2026-04-04T14-44-ryan-Oxm41G/view
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe changes add code search availability detection and conditional guidance emission while enhancing session finalization to detect LFS pointer stubs and respect a needs-summary marker for artifact regeneration. A new struct field and utility functions support marker-based session re-finalization workflows. Changes
Sequence DiagramsequenceDiagram
participant Daemon
participant Detect as Session Finalize<br/>(Detect)
participant LFS
participant Marker
participant ProcessResult
Daemon->>Detect: Detect(ledgerPath)
Detect->>Detect: Scan session directories
rect rgba(200, 150, 100, 0.5)
Note over Detect,LFS: LFS Stub Check
Detect->>LFS: IsPointerFile(rawPath)
alt LFS Pointer Detected
LFS-->>Detect: true
Detect->>Detect: Log skip, continue
else Real JSONL
LFS-->>Detect: false
Detect->>Detect: Check artifacts
end
end
rect rgba(100, 150, 200, 0.5)
Note over Detect,Marker: Needs-Summary Marker Check
Detect->>Detect: All required artifacts present?
alt Yes
Detect->>Marker: HasNeedsSummaryMarker(sessionDir)
alt Marker Found
Marker-->>Detect: true
Detect->>Detect: Queue session-finalize<br/>with Missing: requiredArtifacts
else No Marker
Marker-->>Detect: false
Detect->>Detect: Treat as finalized
end
end
end
Detect-->>Daemon: WorkItems
Daemon->>ProcessResult: Process finalization result
ProcessResult->>ProcessResult: WriteSessionArtifacts
ProcessResult->>Marker: ClearNeedsSummaryMarker
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/daemon/agentwork/session_finalize.go (1)
673-674: Consider logging marker clear failures for debuggability.The error from
ClearNeedsSummaryMarkeris discarded. WhileClearNeedsSummaryMarkerhandlesos.IsNotExistgracefully, other failures (permission denied, I/O errors) would be silently ignored. If clearing fails, the session would be re-enqueued on the next detect cycle—benign but worth logging to surface file system issues.🔧 Optional: Log warning on failure
// clear .needs-summary marker now that LLM artifacts have replaced stubs -_ = session.ClearNeedsSummaryMarker(payload.SessionDir) +if err := session.ClearNeedsSummaryMarker(payload.SessionDir); err != nil { + h.logger.Warn("failed to clear .needs-summary marker", "session", sessionName, "err", err) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/daemon/agentwork/session_finalize.go` around lines 673 - 674, The call to session.ClearNeedsSummaryMarker(payload.SessionDir) currently discards errors; capture its returned error and, if err != nil and not os.IsNotExist(err), emit a warning log that includes the session directory and the error (e.g., "failed to clear .needs-summary marker for %s: %v"). Use the same logger used elsewhere in session_finalize.go (e.g., the file's logger/processLogger) and keep the existing behavior of ignoring os.IsNotExist errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.json:
- Around line 27-35: Remove the duplicate unguarded "bd prime" hook entry that
runs without the existing guard; either delete this second hook block (the one
with "hooks": [{"command": "bd prime", "type": "command"}], "matcher": "") or
modify it to match the guarded pattern used earlier by wrapping the command with
the same guard (e.g., use the shell presence check used in the prior hook block)
so machines without the bd binary don't error and the command isn't executed
twice.
In `@cmd/ox/agent_prime_xml.go`:
- Around line 79-86: The XML payload contains a literal "<query>" which will be
parsed as a tag; update the string construction in agent_prime_xml.go where the
StringBuilder variable sb is used to write the code-search block (the lines that
call sb.WriteString with the guidance text) so that the literal is escaped
(replace "<query>" with "<query>" or run the whole text through an
XML-escape helper like escapeXML) before writing; ensure both the opening and
closing angle brackets are escaped ("<query>" and "</query>" if
needed) so the produced XML remains valid.
---
Nitpick comments:
In `@internal/daemon/agentwork/session_finalize.go`:
- Around line 673-674: The call to
session.ClearNeedsSummaryMarker(payload.SessionDir) currently discards errors;
capture its returned error and, if err != nil and not os.IsNotExist(err), emit a
warning log that includes the session directory and the error (e.g., "failed to
clear .needs-summary marker for %s: %v"). Use the same logger used elsewhere in
session_finalize.go (e.g., the file's logger/processLogger) and keep the
existing behavior of ignoring os.IsNotExist errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 016faf0f-a6e3-4eae-a657-8280f30dabdc
📒 Files selected for processing (8)
.claude/settings.jsoncmd/ox/agent_prime.gocmd/ox/agent_prime_xml.gointernal/daemon/agentwork/session_finalize.gointernal/daemon/agentwork/session_finalize_detect_test.gointernal/daemon/agentwork/session_finalize_lfs_test.gointernal/prime/types.gointernal/session/summary_marker.go
…kets in code-search block Co-Authored-By: SageOx <ox@sageox.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three issues in the anti-entropy session finalization pipeline:
raw.jsonlwere detected every 5 minutes, enqueued, and immediately failed inBuildPrompt— repeating forever. NowdetectInDircallslfs.IsPointerFile()early and skips them.ox session stopwrites stub artifacts (stats-onlysummary.json,summary.md,session.md) before the daemon can run LLM summarization. The daemon'smissingArtifacts()check saw all files present and skipped the session. NowdetectInDiralso checks for the.needs-summarymarker and re-enqueues for LLM regeneration.ProcessResultclears the marker after successful artifact generation.<code-search status="indexed">XML block in prime output that clearly instructs agents to preferox code searchwhen a code index exists.flowchart TD A[detectInDir scans session] --> B{raw.jsonl exists?} B -->|No| Z[skip] B -->|Yes| C{LFS pointer stub?} C -->|Yes| Z C -->|No| D{Has substantive entries?} D -->|No| Z D -->|Yes| E{All artifacts exist?} E -->|No| F[Enqueue for LLM finalization] E -->|Yes| G{.needs-summary marker?} G -->|Yes| H[Enqueue for LLM re-summarization] G -->|No| I{In cache, not pushed?} I -->|Yes| J[Enqueue upload-only] I -->|No| ZTest plan
TestDetect_SkipsLFSStubSessions— LFS pointer sessions skipped at detectionTestDetect_NeedsSummaryMarkerTriggersRefinalization— stub artifacts with marker enqueued for LLMmake lint— 0 issuesmake test— all passCo-authored-by: SageOx ox@sageox.ai
Summary by CodeRabbit
New Features
Bug Fixes