Accept validated numeric evidence citations - #7
Conversation
| 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) |
There was a problem hiding this 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.
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.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Problem
The local
llama3.2chat path can retrieve relevant reviews but still return:The answer prompt required the model to reproduce long content-derived source IDs such as
[review_ab12…]. Small local models commonly emit short numeric citations such as[1]instead. The validator interpreted that number as an invented source ID and rejected the whole answer.Fix
[1]The fail-closed citation policy remains unchanged: uncited answers and citations that do not map to retrieved evidence are not displayed.
Regression coverage
The new regression was observed failing on the merged code with the same citation-validation message shown in the macOS terminal screenshot. Coverage now includes:
Verification
uv lock --checkpassedgit diff --checkpasseduv buildpassedAPPROVERuntime validation still needed
The real macOS Ollama/
llama3.2path is not available on the development host. After CI, the branch should be pulled and tested with the original questions on the reporter's Mac before merge.Greptile Summary
This PR fixes a regression where small local models (llama3.2) emit short numeric citations like
[1]instead of long content-derived source IDs, causing the citation validator to reject valid answers. The fix presents each retrieved review as a numbered evidence record and maps numeric citations strictly back to their stable source IDs before validation, while retaining the exact source-ID citation path for backward compatibility._format_contextnow emits[{n}]+Source ID: {id}for each record;_validate_and_number_citationsbuilds anevidence_aliasesdict (e.g.{\"1\": \"review-abc…\"}) and resolves every cited token through it before checking against theretrievedset.Confidence Score: 4/5
Safe to merge after runtime validation on the target macOS/Ollama host; the citation logic is correct for all realistic source ID shapes.
The alias-lookup line evidence_aliases.get(token, token) silently favors the evidence-number interpretation whenever a source ID is a pure decimal string that falls within the retrieved count, making backward-compat resolution for that document unreachable in those conditions. Content-derived IDs make this extremely unlikely in practice, but the precedence rule is implicit rather than explicit in the code.
Files Needing Attention: agent.py lines 106-110 — the evidence_aliases.get fallback logic warrants a clarifying comment or an explicit numeric vs. non-numeric branch to make alias precedence intentional rather than incidental.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Model answer] --> B{CITATION_PATTERN.findall} B -->|no citations| C[Return None → CITATION_VALIDATION_MESSAGE] B -->|cited_tokens list| D[For each token] D --> E{token in evidence_aliases?} E -->|yes — numeric citation| F[Resolve to alias source_id] E -->|no — source-ID citation| G[Use token as source_id directly] F --> H{source_id in retrieved?} G --> H H -->|no| C H -->|yes| I[Append to resolved_ids] I --> J{More tokens?} J -->|yes| D J -->|no| K[Deduplicate → ordered_ids] K --> L[Build citation_numbers map] L --> M[CITATION_PATTERN.sub — renumber] M --> N[Return numbered_answer + sources tuple]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Accept validated numeric evidence citati..." | Re-trigger Greptile