Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ By default, runtime review data, embeddings, and prompts are processed by embedd
## Highlights

- **Visual dashboard:** rating metrics, distribution chart, date and rating filters, and review browser.
- **Validated citations:** the model cites stable retrieved source IDs; invented or missing citations fail safely.
- **Validated citations:** short evidence citations map strictly to stable retrieved source IDs; invented or missing citations fail safely.
- **Safe offline state:** analytics still load when Ollama is unavailable, while the app shows exact setup commands instead of crashing.
- **Adaptive CSV upload:** automatically detect common headers, manually map unfamiliar names, and isolate every dataset in content-addressed Chroma storage.
- **Reconciled indexing:** content-derived IDs survive reordering; additions, changed records, and deletions are synchronized safely.
Expand Down Expand Up @@ -173,7 +173,7 @@ The suite covers:
- automatic database and collection isolation;
- adaptive rating, date, and categorical filtering;
- Ollama health states;
- source-ID citation validation and model abstention;
- evidence-alias-to-source citation validation and model abstention;
- all four RAG evaluation metrics;
- deterministic upload storage;
- Streamlit rendering without Ollama.
Expand Down
28 changes: 20 additions & 8 deletions agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
ANSWER_PROMPT = """You are a review analyst.
Answer the question using only the supplied reviews. Do not add facts that are not present.
When evidence is mixed or limited, say so clearly. Every factual claim must cite one or more
retrieved source IDs exactly as shown, for example [review_ab12]. Never invent a source ID.
retrieved evidence numbers exactly as shown, for example [1]. Never cite an evidence number
that is not supplied. Source IDs are validation metadata; do not copy them into the answer.
If the supplied reviews do not answer the question, reply exactly INSUFFICIENT_EVIDENCE.

Question:
Expand Down Expand Up @@ -66,12 +67,12 @@ def _format_context(matches: list[ReviewMatch]) -> str:
("restaurant", "Restaurant"),
("country", "Country"),
)
for match in matches:
for evidence_number, match in enumerate(matches, start=1):
metadata = match.document.metadata
source_id = str(metadata.get("source_id") or match.document.id or "")
if not source_id:
raise ValueError("retrieved review is missing a source ID")
lines = [f"[{source_id}]"]
lines = [f"[{evidence_number}]", f"Source ID: {source_id}"]
for key, label in labels:
value = metadata.get(key)
if value is not None:
Expand All @@ -87,24 +88,35 @@ def _validate_and_number_citations(
matches: list[ReviewMatch],
) -> tuple[str, tuple[CitedReview, ...]] | None:
retrieved: dict[str, ReviewMatch] = {}
for match in matches:
evidence_aliases: dict[str, str] = {}
for evidence_number, match in enumerate(matches, start=1):
source_id = str(
match.document.metadata.get("source_id") or match.document.id or ""
)
if not source_id:
return None
retrieved[source_id] = match
evidence_aliases[str(evidence_number)] = source_id

cited_ids = CITATION_PATTERN.findall(answer)
if not cited_ids or any(source_id not in retrieved for source_id in cited_ids):
cited_tokens = CITATION_PATTERN.findall(answer)
if not cited_tokens:
return None

ordered_ids = list(dict.fromkeys(cited_ids))
resolved_ids: list[str] = []
for token in cited_tokens:
source_id = evidence_aliases.get(token, token)
if source_id not in retrieved:
return None
resolved_ids.append(source_id)
Comment on lines +106 to +110

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 Numeric source ID shadows evidence alias

evidence_aliases.get(token, token) always returns the alias-mapped source ID when token is a decimal string that falls within the count of retrieved documents. If a source ID happens to be a pure decimal string (e.g. "3") and the model emits [3] intending backward-compat citation of that document, the lookup resolves instead to the third retrieved record — a different document — whenever three or more records were fetched. Additionally, the resolved document changes when the number of retrieved results crosses the threshold, making behaviour query-dependent. The scenario is unlikely given content-derived IDs, but worth a comment or guard (e.g. if token.isdigit(): ... else: ...) to make the precedence rule explicit.

Prompt To Fix With AI
This is a comment left during a code review.
Path: agent.py
Line: 106-110

Comment:
**Numeric source ID shadows evidence alias**

`evidence_aliases.get(token, token)` always returns the alias-mapped source ID when `token` is a decimal string that falls within the count of retrieved documents. If a source ID happens to be a pure decimal string (e.g. `"3"`) and the model emits `[3]` intending backward-compat citation of that document, the lookup resolves instead to the _third_ retrieved record — a different document — whenever three or more records were fetched. Additionally, the resolved document changes when the number of retrieved results crosses the threshold, making behaviour query-dependent. The scenario is unlikely given content-derived IDs, but worth a comment or guard (e.g. `if token.isdigit(): ... else: ...`) to make the precedence rule explicit.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Evidence aliases intentionally take precedence for decimal bracket tokens. Application-generated source IDs are always content-derived with the review_ prefix, so a pure numeric source ID cannot occur in this runtime. Exact-ID backward compatibility therefore applies to the supported prefixed IDs; interpreting [3] as evidence record 3 is the unambiguous public contract. No code change needed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Understood — with the review_ prefix as an invariant on all application-generated source IDs, a numeric collision is structurally impossible at runtime, so the precedence is safe and the public contract is clear.

Worth noting for anyone reading the code cold: a one-line comment such as

# Source IDs are always prefixed (e.g. "review_…"); plain decimal tokens are
# unambiguously evidence-alias references, so alias lookup takes intentional precedence.
source_id = evidence_aliases.get(token, token)

would make that invariant self-documenting without requiring any logic change. But that's optional — happy to close this if you'd rather leave it as-is.

Shall I remember this as a rule: "source IDs in this project always carry a review_ prefix, so numeric alias precedence in evidence_aliases.get is intentional and needs no guard"?

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.


ordered_ids = list(dict.fromkeys(resolved_ids))
citation_numbers = {
source_id: number for number, source_id in enumerate(ordered_ids, start=1)
}
numbered_answer = CITATION_PATTERN.sub(
lambda match: f"[{citation_numbers[match.group(1)]}]",
lambda match: (
f"[{citation_numbers[evidence_aliases.get(match.group(1), match.group(1))]}]"
),
answer,
)
sources = tuple(
Expand Down
2 changes: 1 addition & 1 deletion docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Runtime review data crosses these boundaries:
2. **Identity layer:** hashes normalized record content and creates an order-independent dataset fingerprint.
3. **Storage layer:** derives a separate Chroma database directory and collection from the dataset fingerprint. Re-indexing reconciles additions, changed records, and deletions.
4. **Retrieval layer:** performs semantic search, then applies deterministic rating, date, sentiment, restaurant, and country filters.
5. **Answer layer:** supplies retrieved source IDs to Ollama. An answer is returned only when every bracketed source ID was retrieved. Valid source IDs are converted to reader-facing citation numbers after validation.
5. **Answer layer:** supplies numbered evidence records and their stable source IDs to Ollama. An answer is returned only when every bracketed evidence number maps to a retrieved record. Exact source-ID citations remain accepted for backward compatibility; validated references are rendered as reader-facing citation numbers.
6. **Evaluation layer:** runs the curated cases in `local_ai_agent/data/rag_cases.json` and reports retrieval recall, citation correctness, reference-grounded answer faithfulness, and abstention accuracy.

## Storage identity
Expand Down
2 changes: 1 addition & 1 deletion docs/architecture.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
65 changes: 64 additions & 1 deletion tests/test_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,73 @@ def test_builds_grounded_prompt_and_returns_cited_sources(self) -> None:
self.assertEqual(len(result.sources), 1)
self.assertEqual(result.sources[0].citation_number, 1)
self.assertEqual(result.sources[0].document.id, "review-1")
self.assertIn("[review-1]", model.prompts[0])
self.assertIn("[1]", model.prompts[0])
self.assertIn("Source ID: review-1", model.prompts[0])
self.assertIn("Great crust Crisp and flavorful.", model.prompts[0])
self.assertIn("only the supplied reviews", model.prompts[0])

def test_accepts_numeric_citation_for_the_matching_retrieved_review(self) -> None:
document = Document(
page_content="Phenomenal crust Crispy and flavorful.",
metadata={"source_id": "review-long-content-hash"},
id="review-long-content-hash",
)

result = answer_question(
"What did the review say about the phenomenal crust?",
vector_store=FakeStore([(document, 0.08)]),
model=FakeModel("The crust was crispy and flavorful [1]."),
)

self.assertEqual(result.answer, "The crust was crispy and flavorful [1].")
self.assertEqual(len(result.sources), 1)
self.assertEqual(result.sources[0].document.id, "review-long-content-hash")

def test_numeric_citations_resolve_to_the_correct_multiple_reviews(self) -> None:
first = Document(
page_content="The crust was crispy.",
metadata={"source_id": "review-a"},
id="review-a",
)
second = Document(
page_content="The crust was thin.",
metadata={"source_id": "review-b"},
id="review-b",
)

result = answer_question(
"What did guests say about the crust?",
vector_store=FakeStore([(first, 0.08), (second, 0.1)]),
model=FakeModel(
"One guest called it thin [2]; another called it crispy [1]."
),
)

self.assertEqual(
result.answer,
"One guest called it thin [1]; another called it crispy [2].",
)
self.assertEqual(
[source.document.id for source in result.sources],
["review-b", "review-a"],
)

def test_rejects_numeric_citation_outside_the_retrieved_set(self) -> None:
document = Document(
page_content="Phenomenal crust Crispy and flavorful.",
metadata={"source_id": "review-long-content-hash"},
id="review-long-content-hash",
)

result = answer_question(
"What did the review say about the phenomenal crust?",
vector_store=FakeStore([(document, 0.08)]),
model=FakeModel("Unsupported statement [2]."),
)

self.assertEqual(result.answer, CITATION_VALIDATION_MESSAGE)
self.assertEqual(result.sources, ())

def test_rejects_citations_that_were_not_retrieved(self) -> None:
document = Document(
page_content="Great crust Crisp and flavorful.",
Expand Down