Skip to content

Fix: show outdated comments instead of hiding them #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
heiko-braun merged 1 commit into from
May 19, 2026
Merged

Fix: show outdated comments instead of hiding them #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions internal/cli/review.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func runReview(port int, branchOverride, serverURL string, statusOnly, debug boo

// 5. --status mode.
if statusOnly {
return printReviewStatus(client, client)
return printReviewStatus(client)
}

// 6. Index documents from local filesystem.
Expand All @@ -110,7 +110,7 @@ func runReview(port int, branchOverride, serverURL string, statusOnly, debug boo

// 8. Create local server backed by remote client.
srv := review.NewServer(
client, client, docIndex, cfg,
client, docIndex, cfg,
repo.Root, repo.Root,
sourceBranch, filepath.Base(repo.Root), userEmail,
debug,
Expand All @@ -136,7 +136,7 @@ func runReview(port int, branchOverride, serverURL string, statusOnly, debug boo
return nil
}

func printReviewStatus(store review.ReviewStore, syncer review.ReviewSyncer) error {
func printReviewStatus(store review.ReviewStore) error {
openReviews, err := store.ListOpenReviews()
if err != nil {
return fmt.Errorf("listing open reviews: %w", err)
Expand All @@ -154,16 +154,9 @@ func printReviewStatus(store review.ReviewStore, syncer review.ReviewSyncer) err
}
}

pending, _ := syncer.HasPendingChanges()

fmt.Printf("Open reviews: %d\n", len(openReviews))
fmt.Printf("Open threads: %d\n", openThreads)
fmt.Printf("Total threads: %d\n", len(allThreads))
if pending {
fmt.Printf("Pending changes: yes\n")
} else {
fmt.Printf("Pending changes: no\n")
}

return nil
}
Expand Down
32 changes: 5 additions & 27 deletions internal/review/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,6 @@ type ReviewStore interface {
CreateReview(title string, documents []string, sourceRef string) (*Review, error)
}

// ReviewSyncer is the interface for sync operations.
type ReviewSyncer interface {
SyncAll() error
Publish() error
HasPendingChanges() (bool, error)
}

// --- Request types ---

// CreateReviewRequest is the JSON body for POST /api/reviews.
Expand Down Expand Up @@ -71,24 +64,9 @@ type DocumentDetail struct {

// StatusResponse is returned by GET /api/status.
type StatusResponse struct {
RepoName string `json:"repo_name"`
Branch string `json:"branch"`
PendingChanges bool `json:"pending_changes"`
OpenReviews int `json:"open_reviews"`
OpenThreads int `json:"open_threads"`
TotalThreads int `json:"total_threads"`
}

// SyncResponse is returned by POST /api/sync.
type SyncResponse struct {
OK bool `json:"ok"`
Message string `json:"message,omitempty"`
Error string `json:"error,omitempty"`
}

// PublishResponse is returned by POST /api/publish.
type PublishResponse struct {
OK bool `json:"ok"`
Message string `json:"message,omitempty"`
Error string `json:"error,omitempty"`
RepoName string `json:"repo_name"`
Branch string `json:"branch"`
OpenReviews int `json:"open_reviews"`
OpenThreads int `json:"open_threads"`
TotalThreads int `json:"total_threads"`
}
20 changes: 3 additions & 17 deletions internal/review/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,10 @@ import (
"time"
)

// Compile-time interface checks.
var (
_ ReviewStore = (*Client)(nil)
_ ReviewSyncer = (*Client)(nil)
)
// Compile-time interface check.
var _ ReviewStore = (*Client)(nil)

// Client is an HTTP client that implements ReviewStore and ReviewSyncer
// Client is an HTTP client that implements ReviewStore
// by delegating to the reviewd API.
type Client struct {
baseURL string // e.g. "http://localhost:5100"
Expand Down Expand Up @@ -248,17 +245,6 @@ func (c *Client) CreateReview(title string, documents []string, sourceRef string
return &rev, nil
}

// --- ReviewSyncer implementation ---

// SyncAll is a no-op — data lives on the server.
func (c *Client) SyncAll() error { return nil }

// Publish is a no-op — mutations are sent immediately.
func (c *Client) Publish() error { return nil }

// HasPendingChanges always returns false — no local queue.
func (c *Client) HasPendingChanges() (bool, error) { return false, nil }

// --- Helpers ---

func newUUIDMust() string {
Expand Down
78 changes: 29 additions & 49 deletions internal/review/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
// Server serves the review UI and JSON API.
type Server struct {
store ReviewStore
syncer ReviewSyncer
docIndex *DocIndex
config ReviewConfig
docsRoot string
Expand All @@ -35,18 +34,16 @@ type Server struct {
logger *log.Logger
}

// NewServer creates a Server wired to the given store, syncer, and document index.
// NewServer creates a Server wired to the given store and document index.
func NewServer(
store ReviewStore,
syncer ReviewSyncer,
docIndex *DocIndex,
config ReviewConfig,
docsRoot, repoRoot, sourceBranch, repoName, userEmail string,
debug bool,
) *Server {
s := &Server{
store: store,
syncer: syncer,
docIndex: docIndex,
config: config,
docsRoot: docsRoot,
Expand Down Expand Up @@ -92,8 +89,6 @@ func (s *Server) routes() {
s.mux.HandleFunc("/api/reviews", s.handleReviews)
s.mux.HandleFunc("/api/threads", s.handleThreads)
s.mux.HandleFunc("/api/threads/", s.handleThreadAction)
s.mux.HandleFunc("/api/sync", s.handleSync)
s.mux.HandleFunc("/api/publish", s.handlePublish)
s.mux.HandleFunc("/api/status", s.handleStatus)
}

Expand Down Expand Up @@ -180,6 +175,19 @@ func (s *Server) handleDocumentDetail(w http.ResponseWriter, r *http.Request) {

threads, _ := s.store.ListThreadsByDocument(docPath)

// Compute outdated status for each thread.
textContent := stripHTMLToText(htmlContent)
for i := range threads {
t := &threads[i]
if t.Anchor.FileHash == fileHash {
continue // hash matches, offsets are valid
}
if t.Anchor.Excerpt != "" && strings.Contains(textContent, t.Anchor.Excerpt) {
continue // excerpt still found via fallback
}
t.Outdated = true
}

detail := DocumentDetail{
Path: docPath,
Title: doc.Title,
Expand Down Expand Up @@ -338,42 +346,6 @@ func (s *Server) handleThreadAction(w http.ResponseWriter, r *http.Request) {
}
}

func (s *Server) handleSync(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
return
}

err := s.syncer.SyncAll()
if err != nil {
writeJSONResponse(w, SyncResponse{OK: false, Error: err.Error()})
return
}

// Re-index documents after sync.
newIndex, indexErr := IndexDocuments(s.docsRoot, s.config.DocumentPaths)
if indexErr == nil {
s.docIndex = newIndex
}

writeJSONResponse(w, SyncResponse{OK: true, Message: "sync complete"})
}

func (s *Server) handlePublish(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
return
}

err := s.syncer.Publish()
if err != nil {
writeJSONResponse(w, PublishResponse{OK: false, Error: err.Error()})
return
}

writeJSONResponse(w, PublishResponse{OK: true, Message: "published successfully"})
}

func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet {
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
Expand All @@ -382,7 +354,6 @@ func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) {

openReviews, _ := s.store.ListOpenReviews()
allThreads, _ := s.store.ListAllThreads()
pending, _ := s.syncer.HasPendingChanges()

openThreads := 0
for _, t := range allThreads {
Expand All @@ -392,12 +363,11 @@ func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) {
}

writeJSONResponse(w, StatusResponse{
RepoName: s.repoName,
Branch: s.sourceBranch,
PendingChanges: pending,
OpenReviews: len(openReviews),
OpenThreads: openThreads,
TotalThreads: len(allThreads),
RepoName: s.repoName,
Branch: s.sourceBranch,
OpenReviews: len(openReviews),
OpenThreads: openThreads,
TotalThreads: len(allThreads),
})
}

Expand All @@ -414,6 +384,16 @@ func httpError(w http.ResponseWriter, msg string, err error) {
http.Error(w, fmt.Sprintf("%s: %v", msg, err), http.StatusInternalServerError)
}

// stripHTMLToText removes HTML tags and returns the text content,
// approximating the browser's element.textContent for anchor matching.
func stripHTMLToText(html string) string {
// Remove all HTML tags.
stripped := regexp.MustCompile(`<[^>]*>`).ReplaceAllString(html, "")
// Collapse whitespace runs the way textContent does.
stripped = strings.Join(strings.Fields(stripped), " ")
return stripped
}

// renderReviewMarkdown renders markdown to HTML using goldmark, adding
// data-paragraph-index attributes to <p> tags for anchor positioning.
func renderReviewMarkdown(source []byte) (string, error) {
Expand Down
12 changes: 12 additions & 0 deletions internal/review/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ type Anchor struct {
// Excerpt is the selected text. Used for display and as a fallback
// to re-locate the annotation when the file has changed.
Excerpt string `json:"excerpt"`

// ContextBefore is ~300 chars of rendered text preceding the selection,
// captured at creation time for display when the anchor becomes outdated.
ContextBefore string `json:"context_before,omitempty"`

// ContextAfter is ~300 chars of rendered text following the selection,
// captured at creation time for display when the anchor becomes outdated.
ContextAfter string `json:"context_after,omitempty"`
}

// ThreadStatus represents the resolution status of a review thread.
Expand Down Expand Up @@ -148,6 +156,10 @@ type Thread struct {

// UpdatedAt is the RFC 3339 timestamp when the thread was last modified.
UpdatedAt string `json:"updated_at,omitempty"`

// Outdated is a computed field (not persisted) indicating the anchor
// can no longer be resolved in the current document content.
Outdated bool `json:"outdated,omitempty"`
}

// Comment is a single entry in a thread's discussion.
Expand Down
Loading
Loading