Skip to content

Improve feedback-driven recall ranking#70

Open
Z-M-Huang wants to merge 2 commits into
mainfrom
feedback-driven-recall-hardcases
Open

Improve feedback-driven recall ranking#70
Z-M-Huang wants to merge 2 commits into
mainfrom
feedback-driven-recall-hardcases

Conversation

@Z-M-Huang

@Z-M-Huang Z-M-Huang commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

What

  • persist identifier-rich recall text and normalized identifier tokens for fragments, claims, facts, and dreams
  • add deterministic, conjunctive exact-anchor matching to the recall reranker while preserving the semantic and keyword RRF path
  • prevent broad identifier-like terms from acting as high-precision anchors, and keep authority/currentness behavior scoped to eligible matches
  • filter related dreams using source and significant-term overlap
  • add a six-case feedback hardcase seed plus evaluation support for evidence and dream references

Why

The feedback-driven candidate regressed because identifier metadata was not consistently searchable and broad terms could be promoted as exact project identifiers. The earlier fix recovered most of the loss, but still remained below the clean baseline. This change targets those failure modes without adding an LLM call to the online recall path.

Evaluation

Two complete 2,727-case runs were compared with the clean baseline:

Metric Clean baseline Two-run mean Delta
Recall@K 0.971092 0.971367 +0.000275
MRR 0.930292 0.930373 +0.000081
nDCG 0.930663 0.930547 -0.000116
Required rank-1 0.900257 0.900623 +0.000367

The result is practical parity with small gains in Recall@K, MRR, and required rank-1; it is not a uniform win because nDCG remains 0.000116 below the clean baseline. Both runs scored all 2,727 cases with zero unmapped source references.

The targeted six-case hardcase suite reached 1.0 for context Recall@K, MRR, nDCG, and required rank-1. Known limits remain: source-type projection can make typed judgments look better than user-visible fragments, and the bad related dream is demoted rather than fully excluded.

Artifacts:

  • tests/eval/runs/public_rag_3axis_5k_v1_after_ranking_anchors_v1/summary.json
  • tests/eval/runs/public_rag_3axis_5k_v1_after_ranking_anchors_v1_repeat_01/summary.json
  • tests/eval/runs/feedback_recall_hardcases_v1_after_ranking_anchors_v1/summary.json
  • tests/eval/runs/compare_public_rag_3axis_5k_clean_vs_ranking_anchors_v1/comparison.json
  • tests/eval/runs/compare_public_rag_3axis_5k_clean_vs_ranking_anchors_v1_repeat_01/comparison.json

Risk

  • Conjunctive hard anchors can reduce recall when a query contains multiple true identifiers but a relevant item stores only a subset. Mitigation: hard-anchor gating applies only to the high-precision class and falls back to the existing RRF ordering when no candidate satisfies it.
  • Identifier normalization can still treat some all-letter hexadecimal strings as anchors. Mitigation: coverage includes mixed identifier/path/version cases, and a future AI-assisted anchor classifier can be evaluated in shadow mode before it affects ranking.
  • Legacy records need the wider candidate fetch path until recall fields are backfilled. Mitigation: the reranker overfetches before applying its final limit.

Checks

  • repository pre-push CI
  • line-count lint
  • full tracked Go package tests
  • coverage: 90.1% (required: 90.0%)
  • go test ./cmd/... ./internal/...
  • git diff --check

Follow-up

AI-assisted anchor extraction is intentionally not included here. The next proposal can compare an exact-span AI classifier as a soft ranking signal, first in offline/shadow evaluation, while deterministic parsing remains the hard safety boundary.

Summary by CodeRabbit

  • New Features

    • Improved recall for references such as pull requests, issues, commits, versions, and file paths.
    • Added identifier-aware filtering and ranking to surface more relevant facts, claims, and fragments.
    • Related dreams are now filtered, reranked, and limited for greater relevance.
    • Added recall metadata to supported search results.
    • Added a feedback-recall hardcase evaluation seed preset.
  • Bug Fixes

    • Improved evidence scoring fallback when fragment evidence references are empty.
    • Added authority-aware ranking to reduce less authoritative matches when stronger sources exist.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds identifier-aware recall data and ranking, Neo4j v2 recall indexes with backfills, feedback recall evaluation seeds, fragment evidence fallback handling, and related-dream filtering and reranking.

Changes

Identifier-aware recall platform

Layer / File(s) Summary
Identifier extraction and anchor matching
internal/recallident/*
Adds identifier extraction, normalization, overlap, hard-gating, ranking anchors, and strong-anchor matching with tests.
Recall fields and v2 search indexes
internal/domain/*, internal/service/{claimservice,dreamservice,factservice,fragmentservice}/..., internal/storage/neo4j/schema.go, internal/tools/keywordsearch/*
Computes and persists recall_text and identifier_tokens, exposes them through reads, creates v2 full-text indexes, and backfills missing values.
Identifier-aware recall and ranking
internal/service/recallservice/*
Adds identifier gating, anchor-based tiering and specificity scoring, authority suppression, typed matching, and relocated RRF helpers.
Evidence fallback and feedback hardcases
internal/evalharness/*, cmd/eval-seedgen/*
Uses fragment context references when mapped fragment evidence is empty and adds a selectable six-case feedback recall hardcase seed generator.
Related dream filtering and reranking
internal/tools/registry/*
Overfetches related dreams, filters and scores them using statuses, identifiers, source references, and lexical overlap, then caps the results.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RecallMemory
  participant RecallService
  participant Neo4j
  participant DreamReranker
  RecallMemory->>RecallService: recall query
  RecallService->>Neo4j: retrieve recall hits
  RecallMemory->>Neo4j: overfetch related dreams
  Neo4j-->>RecallMemory: candidate dreams
  RecallMemory->>DreamReranker: rerankRelatedDreams(query, hits, dreams)
  DreamReranker-->>RecallMemory: filtered and capped related dreams
Loading

Possibly related PRs

Suggested labels: gvi:ready-for-approval

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.95% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: improved recall ranking driven by feedback and identifier-aware scoring.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feedback-driven-recall-hardcases

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gitvibe-for-github gitvibe-for-github Bot added the gvi:reviewing GitVibe is reviewing a pull request. label Jul 10, 2026

@gitvibe-for-github gitvibe-for-github Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GitVibe Risk Accepted

Status: blocked
Risk: accepted for matching context

Z-M-Huang accepted this prompt-injection input risk for matching review-matrix context.
GitVibe replaced the previous blocked result details to keep this thread readable.

Accepted Risk

Z-M-Huang accepted this prompt-injection input risk for matching context.
Accepted at: 2026-07-10T00:28:27.775Z
Accepted workflow run: 29059688967
Accepted workflow attempt: 2
Accepted stages: review-matrix
Artifact title/body SHA: 27d9169c051a0968ee5a27450941cf529f48dba27a8216e5ced5bbf3f73f4042
Pull request head SHA: 9a46c0fabb278ea2d63b8653ff3faa814d2b0e60

@gitvibe-for-github gitvibe-for-github Bot added gvi:blocked GitVibe is blocked by missing or contradictory information. and removed gvi:reviewing GitVibe is reviewing a pull request. labels Jul 10, 2026
@Z-M-Huang Z-M-Huang added the git-vibe:accept-risk Trusted actor accepted prompt-injection risk for one GitVibe rerun. label Jul 10, 2026
@gitvibe-for-github gitvibe-for-github Bot added gvi:reviewing GitVibe is reviewing a pull request. and removed gvi:blocked GitVibe is blocked by missing or contradictory information. git-vibe:accept-risk Trusted actor accepted prompt-injection risk for one GitVibe rerun. labels Jul 10, 2026

@gitvibe-for-github gitvibe-for-github Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GitVibe Risk Accepted

Status: blocked
Risk: accepted for matching context

Z-M-Huang accepted this prompt-injection input risk for matching review-matrix context.
GitVibe replaced the previous blocked result details to keep this thread readable.

Accepted Risk

Z-M-Huang accepted this prompt-injection input risk for matching context.
Accepted at: 2026-07-10T01:26:29.973Z
Accepted workflow run: 29059688967
Accepted workflow attempt: 3
Accepted stages: review-matrix
Artifact title/body SHA: 27d9169c051a0968ee5a27450941cf529f48dba27a8216e5ced5bbf3f73f4042
Pull request head SHA: 9a46c0fabb278ea2d63b8653ff3faa814d2b0e60

@gitvibe-for-github gitvibe-for-github Bot added gvi:blocked GitVibe is blocked by missing or contradictory information. and removed gvi:reviewing GitVibe is reviewing a pull request. labels Jul 10, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a46c0fabb

ℹ️ 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 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 address that feedback".

Comment on lines +450 to +451
if len(queryHardAnchors) > 0 && !recallident.SatisfiesAnchors(queryHardAnchors, m.Content, m.RecallText) {
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Fall back when no fragment satisfies all hard anchors

When a query contains hard anchors but none of the overfetched fragment candidates contains every anchor, this branch drops every fragment candidate. For example, a low-limit query that mentions both a PR and a commit can retrieve only partial/legacy matches in the branch pool; instead of returning the best RRF-ranked fallback, recall returns only tier hits or an empty fragment list. Please first check whether any candidate satisfies the hard anchors, and only apply this filter when that set is non-empty.

Useful? React with 👍 / 👎.

"fact_predicate_idx",
},
{
"CREATE FULLTEXT INDEX fragment_recall_v2_idx IF NOT EXISTS FOR (sf:SourceFragment) ON EACH [sf.content, sf.source, sf.idempotency_key, sf.recall_text]",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include metadata-only anchors in imported eval rows

The keyword branch now queries this v2 index, but it still omits metadata_json. Newly direct-imported eval corpus rows are written by internal/evalharness/direct_import.go without recall_text, and several new hardcases put required anchors only in CorpusItem.Metadata (for example PR/commit/tool identifiers), so those rows never expose the anchor terms to the index until an external backfill happens. This makes the new metadata hardcases miss the required fragment and favor decoys whose body text contains the terms; either index metadata_json here or populate recall_text during direct import.

Useful? React with 👍 / 👎.

Comment on lines +249 to +250
if strings.Contains(token, "/") || strings.Contains(token, ":") || strings.Contains(token, "_") {
return true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Do not treat broad punctuation terms as typed anchors

Typed fact/claim recall calls SatisfiesStrongAnchors, so this rule makes any slash/colon/underscore token an exact gate even when it is ordinary prose. For queries such as TCR/CD3 or U.S. economy, full-text search can retrieve a relevant fact whose text normalizes the punctuation differently (TCR and CD3, US economy), but the post-filter drops it because Extract cannot recreate the exact slash/dotted token. This broad-token gating was avoided for fragment hard anchors, but facts and claims still lose otherwise valid recall results.

Useful? React with 👍 / 👎.

Comment on lines +906 to +907
recall_text: $recallText,
identifier_tokens: $identifierTokens

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Merge anchors when confirming an existing fact

This only initializes recall fields when a new Fact is created. When handleSingleCurrent reuses an existing same-object fact via linkClaimToExistingFact, the confirming claim is marked superseded but its recall_text and identifier_tokens are never merged into the existing Fact; if that claim carried a new metadata/source identifier, the claim is no longer eligible for validated-claim recall and the Fact cannot be found by the new anchor. Please update the existing Fact's recall fields in the reuse path as well.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (4)
internal/tools/registry/recall_dream_filter_test.go (1)

30-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add order assertions to verify reranking behavior.

The test verifies which dreams are included/excluded but doesn't assert their order. For a reranking function, the order is a key behavior — a wrong order with the right set would pass the current test but produce incorrect results in production.

✨ Proposed addition
 	for _, want := range []string{"dream-related-1", "dream-related-2", "dream-related-3"} {
 		if !gotIDs[want] {
 			t.Fatalf("related dreams missing %s: %+v", want, gotIDs)
 		}
 	}
 	for _, notWant := range []string{"dream-related-4", "dream-unrelated", "dream-stale"} {
 		if gotIDs[notWant] {
 			t.Fatalf("related dreams included %s: %+v", notWant, gotIDs)
 		}
 	}
+	wantOrder := []string{"dream-related-1", "dream-related-2", "dream-related-3"}
+	for i, wantID := range wantOrder {
+		if got[i].DreamID != wantID {
+			t.Fatalf("related dreams[%d] = %s; want %s", i, got[i].DreamID, wantID)
+		}
+	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tools/registry/recall_dream_filter_test.go` around lines 30 - 43,
Add assertions in the recall/reranking test after collecting got so it verifies
the returned dreams appear in the expected reranked order, not merely the
expected set. Compare each result’s DreamID against the intended ordered
sequence and fail with the actual order when mismatched, while retaining the
existing inclusion/exclusion checks.
internal/service/claimservice/create.go (1)

434-470: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Non-deterministic parts/token ordering from map iteration.

for key, value := range classification (Line 447) and for key, value := range fragment.Metadata (Line 462) iterate over Go maps, whose order is randomized per run. This means recallText/identifierTokens generated for logically identical claims can differ across create calls or backfill re-runs, undermining the deterministic-recall goal called out in the PR objectives. Functionally the deduped token set is likely unaffected (via the MergeTokens collector), but the persisted recall_text string is not reproducible, which will also make any exact-string assertions in tests or backfill diffing flaky.

♻️ Proposed fix: sort map keys before iterating
+	classKeys := make([]string, 0, len(classification))
+	for key := range classification {
+		classKeys = append(classKeys, key)
+	}
+	sort.Strings(classKeys)
+	for _, key := range classKeys {
+		parts = append(parts, key, fmt.Sprint(classification[key]))
+	}
-	for key, value := range classification {
-		parts = append(parts, key, fmt.Sprint(value))
-	}
 	tokenGroups := make([][]string, 0, len(fragments)+1)
 	for _, fragment := range fragments {
 		if fragment == nil {
 			continue
 		}
 		parts = append(parts,
 			fragment.Content,
 			fragment.Source,
 			fragment.IdempotencyKey,
 			fragment.RecallText,
 		)
 		parts = append(parts, fragment.Labels...)
-		for key, value := range fragment.Metadata {
-			parts = append(parts, key, fmt.Sprint(value))
-		}
+		metaKeys := make([]string, 0, len(fragment.Metadata))
+		for key := range fragment.Metadata {
+			metaKeys = append(metaKeys, key)
+		}
+		sort.Strings(metaKeys)
+		for _, key := range metaKeys {
+			parts = append(parts, key, fmt.Sprint(fragment.Metadata[key]))
+		}
 		tokenGroups = append(tokenGroups, fragment.IdentifierTokens)
 	}

Requires adding "sort" to imports.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/service/claimservice/create.go` around lines 434 - 470, Make
claimRecallFields deterministic by sorting map keys before appending
classification and fragment metadata entries. Add the sort import, collect and
sort keys for classification and each fragment.Metadata map, then iterate in
sorted order so recallText and identifierTokens are reproducible.
internal/service/dreamservice/service.go (1)

938-950: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cross-file duplicate of the []any[]string filter helper.

stringSliceFromMap here duplicates the same filtering logic as stringSlice in internal/service/factservice/promote_rows.go (lines 51-63). Both were introduced in this PR for the same identifier-tokens decoding need.

Consider extracting a single shared helper (e.g. in a small internal util package) used by both dreamservice and factservice (and any other callers) instead of maintaining parallel implementations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/service/dreamservice/service.go` around lines 938 - 950, Extract the
duplicated []any-to-[]string filtering logic from stringSliceFromMap and
factservice’s stringSlice into one shared internal utility helper, then update
both callers to use it while preserving empty-string filtering and invalid-type
handling.
internal/service/factservice/promote_rows.go (1)

51-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the new stringSlice helper for supportedBy instead of duplicating the same logic.

The new stringSlice helper (lines 51-63) and the inline supportedBy parsing (lines 65-73) are functionally identical.

♻️ Proposed fix
-	var supportedBy []string
-	if raw, ok := row["supported_by"].([]any); ok {
-		supportedBy = make([]string, 0, len(raw))
-		for _, v := range raw {
-			if s, ok := v.(string); ok && s != "" {
-				supportedBy = append(supportedBy, s)
-			}
-		}
-	}
+	supportedBy := stringSlice("supported_by")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/service/factservice/promote_rows.go` around lines 51 - 73, Reuse the
existing stringSlice helper when assigning supportedBy, replacing the duplicated
type assertion and filtering loop with a call using the "supported_by" key.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/storage/neo4j/schema.go`:
- Around line 560-568: Ensure backfillRecallSearchText is not run
unconditionally on every EnsureSchema invocation. Add an idempotent one-time
migration marker, such as a SchemaMigration node keyed by migration name, and
gate the four label backfill queries behind an atomic check/create so completed
backfills are skipped on subsequent startups; preserve retry behavior if the
backfill fails.
- Around line 573-585: Update the schema migration entry identified by
“sourcefragment_recall_text” and the corresponding Claim, Fact, and Dream
backfills to recompute recall_text and identifier_tokens through the shared
recallident builders used by write paths, rather than inline Cypher and empty
arrays. Implement a real migration that loads legacy records, applies the common
helper, and persists both derived fields consistently with new records.

In `@internal/tools/registry/recall_dream_filter.go`:
- Around line 34-35: Update the identifierOverlap calculation in the recall
filter to exclude the metadata fields dream.CycleRunID and dream.GeneratorModel
from recallident.OverlapText, while retaining the substantive dream content
fields.

---

Nitpick comments:
In `@internal/service/claimservice/create.go`:
- Around line 434-470: Make claimRecallFields deterministic by sorting map keys
before appending classification and fragment metadata entries. Add the sort
import, collect and sort keys for classification and each fragment.Metadata map,
then iterate in sorted order so recallText and identifierTokens are
reproducible.

In `@internal/service/dreamservice/service.go`:
- Around line 938-950: Extract the duplicated []any-to-[]string filtering logic
from stringSliceFromMap and factservice’s stringSlice into one shared internal
utility helper, then update both callers to use it while preserving empty-string
filtering and invalid-type handling.

In `@internal/service/factservice/promote_rows.go`:
- Around line 51-73: Reuse the existing stringSlice helper when assigning
supportedBy, replacing the duplicated type assertion and filtering loop with a
call using the "supported_by" key.

In `@internal/tools/registry/recall_dream_filter_test.go`:
- Around line 30-43: Add assertions in the recall/reranking test after
collecting got so it verifies the returned dreams appear in the expected
reranked order, not merely the expected set. Compare each result’s DreamID
against the intended ordered sequence and fail with the actual order when
mismatched, while retaining the existing inclusion/exclusion checks.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f2bbbeb2-de54-4ffd-bec1-87f74090d985

📥 Commits

Reviewing files that changed from the base of the PR and between 42475df and 9a46c0f.

📒 Files selected for processing (42)
  • cmd/eval-seedgen/feedback_recall_hardcases.go
  • cmd/eval-seedgen/main.go
  • internal/domain/claim.go
  • internal/domain/dream.go
  • internal/domain/fact.go
  • internal/domain/fragment.go
  • internal/evalharness/runner_evidence_fallback_test.go
  • internal/evalharness/runner_test.go
  • internal/evalharness/score.go
  • internal/recallident/identifier.go
  • internal/recallident/identifier_test.go
  • internal/service/claimservice/create.go
  • internal/service/claimservice/read.go
  • internal/service/claimservice/support.go
  • internal/service/dreamservice/recall_fields.go
  • internal/service/dreamservice/service.go
  • internal/service/dreamservice/service_test.go
  • internal/service/factservice/promote.go
  • internal/service/factservice/promote_rows.go
  • internal/service/factservice/read.go
  • internal/service/fragmentservice/create.go
  • internal/service/fragmentservice/list.go
  • internal/service/fragmentservice/read.go
  • internal/service/recallservice/recall.go
  • internal/service/recallservice/recall_fragment_limit_test.go
  • internal/service/recallservice/recall_identifier.go
  • internal/service/recallservice/recall_limit_test.go
  • internal/service/recallservice/recall_rerank.go
  • internal/service/recallservice/recall_rerank_test.go
  • internal/service/recallservice/recall_rrf.go
  • internal/service/recallservice/recall_temporal.go
  • internal/service/recallservice/recall_temporal_test.go
  • internal/service/recallservice/recall_test.go
  • internal/service/recallservice/searcher.go
  • internal/storage/neo4j/schema.go
  • internal/tools/keywordsearch/searcher.go
  • internal/tools/keywordsearch/service.go
  • internal/tools/registry/dream_tools_test.go
  • internal/tools/registry/evaluation_tools.go
  • internal/tools/registry/recall_dream_filter.go
  • internal/tools/registry/recall_dream_filter_test.go
  • internal/tools/registry/toolset.go

Comment on lines +560 to +568
if err := s.backfillRecallSearchText(ctx); err != nil {
return err
}

s.logger.Info("Neo4j schema ensured successfully")
return nil
}

func (s *SchemaBootstrapper) backfillRecallSearchText(ctx context.Context) error {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Unconditional full-label scans on every EnsureSchema call.

Unlike the surrounding IF NOT EXISTS index/constraint statements, these four backfill queries always MATCH the entire label (SourceFragment/Claim/Fact/Dream) to evaluate WHERE ... IS NULL, with no marker to skip once already backfilled. If EnsureSchema runs on every service startup/deploy (as its idempotent "ensure" pattern suggests), this adds a full scan of potentially large node sets on every boot.

Consider gating this behind a one-time marker (e.g. a SchemaMigration{name} node) so it only executes once rather than on every bootstrap.

Also applies to: 640-651

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/storage/neo4j/schema.go` around lines 560 - 568, Ensure
backfillRecallSearchText is not run unconditionally on every EnsureSchema
invocation. Add an idempotent one-time migration marker, such as a
SchemaMigration node keyed by migration name, and gate the four label backfill
queries behind an atomic check/create so completed backfills are skipped on
subsequent startups; preserve retry behavior if the backfill fails.

Comment on lines +573 to +585
{
name: "sourcefragment_recall_text",
cypher: `
MATCH (sf:SourceFragment)
WHERE sf.recall_text IS NULL OR sf.identifier_tokens IS NULL
SET sf.recall_text = coalesce(sf.recall_text, trim(
coalesce(sf.content, '') + ' ' +
coalesce(sf.source, '') + ' ' +
coalesce(sf.idempotency_key, '') + ' ' +
reduce(acc = '', label IN coalesce(sf.labels, []) | acc + ' ' + toString(label)) + ' ' +
coalesce(sf.metadata_json, '')
)),
sf.identifier_tokens = coalesce(sf.identifier_tokens, [])`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Look for any other identifier_tokens backfill/reindex mechanism.
rg -nP 'identifier_tokens' --type=go -g '!**/*_test.go' -C2

Repository: markhuangai/dense-mem

Length of output: 159


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the schema file around the cited backfill sections and search for related backfill/reindex logic.
git ls-files | sed -n '1,200p'
printf '\n--- identifier_tokens references ---\n'
rg -n "identifier_tokens|recallident|backfillRecallSearchText|recall_text" internal . -g '!**/*_test.go' -g '!**/*.md' -g '!**/vendor/**' -C 2

Repository: markhuangai/dense-mem

Length of output: 50377


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the cited area in internal/storage/neo4j/schema.go with line numbers.
wc -l internal/storage/neo4j/schema.go
sed -n '520,680p' internal/storage/neo4j/schema.go | cat -n

Repository: markhuangai/dense-mem

Length of output: 6448


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- backfill / reindex references ---\n'
rg -n "backfill|reindex|recompute|refresh.*identifier|identifier_tokens" internal cmd -g '!**/*_test.go' -C 2

printf '\n--- recallident implementation ---\n'
sed -n '1,220p' internal/recallident/identifier.go | cat -n

Repository: markhuangai/dense-mem

Length of output: 250


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- schema backfill line ranges ---\n'
sed -n '568,640p' internal/storage/neo4j/schema.go | cat -n

printf '\n--- recall text builders used on writes ---\n'
sed -n '210,260p' internal/service/fragmentservice/create.go | cat -n
sed -n '450,475p' internal/service/claimservice/create.go | cat -n
sed -n '585,605p' internal/service/factservice/promote.go | cat -n
sed -n '1,40p' internal/service/dreamservice/recall_fields.go | cat -n

Repository: markhuangai/dense-mem

Length of output: 250


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- backfill / reindex references ---'
rg -n "backfill|reindex|recompute|refresh.*identifier|identifier_tokens" internal cmd -g '!**/*_test.go' -C 2 || true

echo
echo '--- recallident implementation ---'
sed -n '1,220p' internal/recallident/identifier.go | cat -n

Repository: markhuangai/dense-mem

Length of output: 32263


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- schema backfill line ranges ---'
sed -n '568,640p' internal/storage/neo4j/schema.go | cat -n

echo
echo '--- recall text builders used on writes ---'
sed -n '220,240p' internal/service/fragmentservice/create.go | cat -n
sed -n '460,470p' internal/service/claimservice/create.go | cat -n
sed -n '590,600p' internal/service/factservice/promote.go | cat -n
sed -n '1,30p' internal/service/dreamservice/recall_fields.go | cat -n

Repository: markhuangai/dense-mem

Length of output: 5755


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- exact identifier_tokens usages outside schema backfill ---'
rg -n "identifier_tokens\s*=" internal cmd -g '!**/*_test.go' -C 1 || true

Repository: markhuangai/dense-mem

Length of output: 1504


Backfill legacy recall fields with the shared builders

internal/storage/neo4j/schema.go:568-637 backfills legacy rows with identifier_tokens = [] instead of extracted tokens, and rebuilds recall_text inline rather than through the shared recallident path used on writes. That leaves pre-existing SourceFragment, Claim, Fact, and Dream rows with permanently weaker search/ranking metadata. Consider a real migration that recomputes both fields from the same helper used for new records.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/storage/neo4j/schema.go` around lines 573 - 585, Update the schema
migration entry identified by “sourcefragment_recall_text” and the corresponding
Claim, Fact, and Dream backfills to recompute recall_text and identifier_tokens
through the shared recallident builders used by write paths, rather than inline
Cypher and empty arrays. Implement a real migration that loads legacy records,
applies the common helper, and persists both derived fields consistently with
new records.

Comment on lines +34 to +35
identifierOverlap := recallident.Overlap(queryIDs, dream.IdentifierTokens) +
recallident.OverlapText(queryIDs, dream.Hypothesis, dream.WhatIf, dream.PossibleOutcome, dream.Rationale, dream.RecallText, dream.CycleRunID, dream.GeneratorModel)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Consider excluding metadata fields from identifier overlap.

dream.CycleRunID and dream.GeneratorModel are metadata, not content. Including them in recallident.OverlapText means a query mentioning a model name (e.g., "gpt-4") would boost all dreams generated by that model regardless of topical relevance, potentially degrading ranking quality.

🔧 Proposed fix
 		identifierOverlap := recallident.Overlap(queryIDs, dream.IdentifierTokens) +
-			recallident.OverlapText(queryIDs, dream.Hypothesis, dream.WhatIf, dream.PossibleOutcome, dream.Rationale, dream.RecallText, dream.CycleRunID, dream.GeneratorModel)
+			recallident.OverlapText(queryIDs, dream.Hypothesis, dream.WhatIf, dream.PossibleOutcome, dream.Rationale, dream.RecallText)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
identifierOverlap := recallident.Overlap(queryIDs, dream.IdentifierTokens) +
recallident.OverlapText(queryIDs, dream.Hypothesis, dream.WhatIf, dream.PossibleOutcome, dream.Rationale, dream.RecallText, dream.CycleRunID, dream.GeneratorModel)
identifierOverlap := recallident.Overlap(queryIDs, dream.IdentifierTokens) +
recallident.OverlapText(queryIDs, dream.Hypothesis, dream.WhatIf, dream.PossibleOutcome, dream.Rationale, dream.RecallText)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tools/registry/recall_dream_filter.go` around lines 34 - 35, Update
the identifierOverlap calculation in the recall filter to exclude the metadata
fields dream.CycleRunID and dream.GeneratorModel from recallident.OverlapText,
while retaining the substantive dream content fields.

@Z-M-Huang Z-M-Huang added the git-vibe:accept-risk Trusted actor accepted prompt-injection risk for one GitVibe rerun. label Jul 10, 2026
@gitvibe-for-github gitvibe-for-github Bot added gvi:reviewing GitVibe is reviewing a pull request. and removed gvi:blocked GitVibe is blocked by missing or contradictory information. git-vibe:accept-risk Trusted actor accepted prompt-injection risk for one GitVibe rerun. labels Jul 10, 2026

@gitvibe-for-github gitvibe-for-github Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GitVibe Risk Accepted

Status: blocked
Risk: accepted for matching context

Z-M-Huang accepted this prompt-injection input risk for matching review-matrix context.
GitVibe replaced the previous blocked result details to keep this thread readable.

Accepted Risk

Z-M-Huang accepted this prompt-injection input risk for matching context.
Accepted at: 2026-07-10T02:34:41.508Z
Accepted workflow run: 29059688967
Accepted workflow attempt: 4
Accepted stages: review-matrix
Artifact title/body SHA: 27d9169c051a0968ee5a27450941cf529f48dba27a8216e5ced5bbf3f73f4042
Pull request head SHA: 9a46c0fabb278ea2d63b8653ff3faa814d2b0e60

@gitvibe-for-github gitvibe-for-github Bot added gvi:blocked GitVibe is blocked by missing or contradictory information. and removed gvi:reviewing GitVibe is reviewing a pull request. labels Jul 10, 2026
@Z-M-Huang Z-M-Huang added the git-vibe:accept-risk Trusted actor accepted prompt-injection risk for one GitVibe rerun. label Jul 10, 2026
@gitvibe-for-github gitvibe-for-github Bot added gvi:reviewing GitVibe is reviewing a pull request. and removed gvi:blocked GitVibe is blocked by missing or contradictory information. git-vibe:accept-risk Trusted actor accepted prompt-injection risk for one GitVibe rerun. labels Jul 10, 2026

@gitvibe-for-github gitvibe-for-github Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GitVibe Review Matrix

Status: blocked
Next state: blocked

GitVibe paused this run for maintainer review.

Questions

  1. AI safety gate failed closed.
    A. Rerun after the safety classifier runtime is healthy, or fix the safety configuration before rerunning.

Next Action

Reply in one comment with question numbers and option letters, or write your own answer for any question.

Result

When present, required fixes are posted as pull request review comments. Full structured output remains in the workflow run summary.
Workflow run: https://github.com/markhuangai/dense-mem/actions/runs/29059688967

Inline comments: 0

Required Fixes

  1. safety gate: AI safety gate failed: Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again. - The AI safety classifier failed while the prompt-injection gate was enabled.

@gitvibe-for-github gitvibe-for-github Bot added gvi:blocked GitVibe is blocked by missing or contradictory information. and removed gvi:reviewing GitVibe is reviewing a pull request. labels Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gvi:blocked GitVibe is blocked by missing or contradictory information.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant