Repository navigation
feat(map): confirm browser locations in reports - #179
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough新增 Changes位置确认功能
Intake AI 处理
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser as 浏览器
participant Picker as LocationConfirmationPicker
participant AMap as 高德地图 API
participant Form as 工作区表单
Browser->>Picker: 用户主动请求当前位置
Picker->>Browser: 获取浏览器坐标
Picker->>AMap: 转换坐标并查询地址
AMap-->>Picker: 返回地址和坐标
Picker->>Form: 用户确认后回填位置
sequenceDiagram
participant Form as FamilyIntakeForm
participant Client as apiSseRequest
participant Route as intake_sessions
participant Gateway as AI Gateway
Form->>Client: 请求草稿或初始审核
Client->>Route: 发送 SSE 请求
Route->>Gateway: 执行 AI 服务调用
Gateway-->>Route: 返回 AI 结果
Route-->>Client: 发送 completed 或 error 事件
Client-->>Form: 返回解析后的结果
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Preview readyThe preview for this build is available at https://pr-179-a8f39993ce82.angui.cg8.site. Temporary demo accounts (all use the password
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/WORKFLOWS.md (1)
216-229: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win将
AMAP_JS_API_KEY的说明移出"Optional preview secret"分组。
AMAP_JS_API_KEY不只用于预览环境。ci.yml在每次前端检查构建时注入它。release.yml在生产发布构建时也注入它。当前文档把这一条紧跟在"Optional preview secret"标题下的AMAP_WEBSERVICE_KEY条目之后，容易让配置者误以为它只影响预览部署，从而遗漏为生产发布配置该密钥。为该条目新增独立的小标题（例如"前端构建密钥"），或明确说明它适用于 CI 检查、预览部署和生产发布这三种构建场景。
🤖 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 @.github/WORKFLOWS.md around lines 216 - 229, 将 .github/WORKFLOWS.md 中的 AMAP_JS_API_KEY 条目移出“Optional preview secret”分组，并为其添加独立的小标题或明确标注其适用于 CI 前端检查构建、预览部署构建和生产发布构建；保留其仅注入前端构建、可公开暴露且不得替代 AMAP_WEBSERVICE_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 `@frontend/.env.example`:
- Line 2: 在 frontend/.env.example 中调整环境变量顺序，将 VITE_AMAP_JS_API_KEY 放到
VITE_API_BASE_URL 之前，保持变量内容不变。
In `@frontend/src/components/LocationConfirmationPicker.tsx`:
- Around line 154-184: Update resolveAddress so that successful address parsing
clears the existing message state before updating the candidate location; keep
the failure path’s existing setMessage behavior unchanged and perform the reset
only after a non-empty formatted address is obtained.
- Around line 95-134: Update loadAmap so every script-loading failure clears
amapPromise and removes the failed script element identified by amapScriptId
before rejecting. Ensure all existing-script and newly-created-script error
paths perform this cleanup, allowing the next loadAmap call to create a fresh
script and retry.
In `@frontend/src/pages/CaseWorkspacePage.tsx`:
- Around line 1383-1392: 为线索表单和地点表单中渲染的两个 LocationConfirmationPicker 添加
key={detail.id}，分别定位到 CaseDetailView
中的两处选择器渲染位置。确保切换案件时选择器重新挂载并重置未确认的候选位置及其内部状态，同时保留现有确认和清除回调行为。
In `@frontend/src/pages/VolunteerWorkspacePage.tsx`:
- Around line 744-763: Update the ClueDraft type and its initialization to
include locationPrecision as "exact" | "approximate" | "". In
LocationConfirmationPicker’s onConfirm, set the selected address and
locationPrecision to "exact"; in onClear, reset both location and precision.
Update the clue submission logic to use clueDraft.locationPrecision, falling
back to "approximate" only for non-empty manually entered locations and null
otherwise.
---
Outside diff comments:
In @.github/WORKFLOWS.md:
- Around line 216-229: 将 .github/WORKFLOWS.md 中的 AMAP_JS_API_KEY 条目移出“Optional
preview secret”分组，并为其添加独立的小标题或明确标注其适用于 CI
前端检查构建、预览部署构建和生产发布构建；保留其仅注入前端构建、可公开暴露且不得替代 AMAP_WEBSERVICE_KEY 的说明。
🪄 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: 4323301c-3d44-4471-a672-46c41a2f5749
📒 Files selected for processing (9)
.github/WORKFLOWS.md.github/workflows/ci.yml.github/workflows/release.ymlfrontend/.env.examplefrontend/README.mdfrontend/src/components/LocationConfirmationPicker.test.tsxfrontend/src/components/LocationConfirmationPicker.tsxfrontend/src/pages/CaseWorkspacePage.tsxfrontend/src/pages/VolunteerWorkspacePage.tsx
| @@ -1 +1,2 @@ | |||
| VITE_API_BASE_URL=/api | |||
| VITE_AMAP_JS_API_KEY= | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
按字母顺序排列环境变量键，消除 dotenv-linter 警告。
静态分析工具报告 VITE_AMAP_JS_API_KEY 应排在 VITE_API_BASE_URL 之前。调整两行的顺序即可消除该警告。
🔧 调整变量顺序
+VITE_AMAP_JS_API_KEY=
VITE_API_BASE_URL=/api
-VITE_AMAP_JS_API_KEY=🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 2-2: [UnorderedKey] The VITE_AMAP_JS_API_KEY key should go before the VITE_API_BASE_URL key
(UnorderedKey)
🤖 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 `@frontend/.env.example` at line 2, 在 frontend/.env.example 中调整环境变量顺序，将
VITE_AMAP_JS_API_KEY 放到 VITE_API_BASE_URL 之前，保持变量内容不变。
Source: Linters/SAST tools
| <LocationConfirmationPicker | ||
| onConfirm={(location) => { | ||
| setClueLocation(location.address); | ||
| setClueLocationPrecision("exact"); | ||
| }} | ||
| onClear={() => { | ||
| setClueLocation(""); | ||
| setClueLocationPrecision(""); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
为位置选择器绑定 detail.id，避免跨案件残留未确认位置。
CaseDetailView 在两处渲染时都没有 key={detail.id}（约第 316 行和第 463 行）。切换所选案件，或在指挥端路由间切换案件，都不会重新挂载 CaseDetailView，只会更新它的 props。clueLocation、place 以及嵌套的 LocationConfirmationPicker 内部状态（candidate、map.current、marker.current）都不会因此重置。
如果用户在案件 A 打开地图选点，得到一个候选位置后没有确认也没有清除，然后切换到案件 B，地图仍会显示案件 A 的候选坐标。此时如果用户点击"确认此位置"，案件 A 的坐标和地址会被写入案件 B 的线索或地点表单字段。在走失人员搜救场景中，这会把错误的位置提交给错误的案件。
为线索表单和地点表单中的 LocationConfirmationPicker 添加 key={detail.id}。案件切换时会重新挂载选择器，清空未确认的候选位置，不影响 CaseDetailView 中其他字段的现有行为。
🔧 按案件重置选择器状态
<LocationConfirmationPicker
+ key={detail.id}
onConfirm={(location) => {
setClueLocation(location.address);
setClueLocationPrecision("exact");
}}
onClear={() => {
setClueLocation("");
setClueLocationPrecision("");
}}
/> <LocationConfirmationPicker
+ key={detail.id}
onConfirm={(location) =>
setPlace({
...place,
address: location.address,
longitude: location.longitude,
latitude: location.latitude,
})
}
onClear={() =>
setPlace({
...place,
address: "",
longitude: null,
latitude: null,
})
}
/>As per path instructions for frontend/**: "Focus on frontend/backend API contract compatibility, authentication state, validation, data exposure, and user-visible failure paths."
Also applies to: 1587-1604
🤖 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 `@frontend/src/pages/CaseWorkspacePage.tsx` around lines 1383 - 1392,
为线索表单和地点表单中渲染的两个 LocationConfirmationPicker 添加 key={detail.id}，分别定位到
CaseDetailView 中的两处选择器渲染位置。确保切换案件时选择器重新挂载并重置未确认的候选位置及其内部状态，同时保留现有确认和清除回调行为。
Source: Path instructions
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
frontend/src/components/LocationConfirmationPicker.test.tsx (1)
236-239: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value提取
navigator.geolocation的 stub 辅助函数。五个测试都重复了同一段
Object.defineProperty(navigator, "geolocation", ...)。请提取一个辅助函数，并在afterEach中恢复原始的navigator.geolocation，避免污染同一文件中的其他测试。♻️ 建议改法
+function stubGeolocation(getCurrentPosition: unknown) { + Object.defineProperty(navigator, "geolocation", { + configurable: true, + value: { getCurrentPosition }, + }); +}🤖 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 `@frontend/src/components/LocationConfirmationPicker.test.tsx` around lines 236 - 239, 在 LocationConfirmationPicker 测试文件中提取统一 stub navigator.geolocation 的辅助函数，替换五个测试中的重复 Object.defineProperty 配置，并在 afterEach 中保存后恢复原始 navigator.geolocation，确保测试间不共享被修改的全局状态。frontend/src/components/LocationConfirmationPicker.tsx (1)
193-204: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value把对
candidateRef的写入移出 state updater。
setCandidate的 updater 必须是纯函数。第 202 行在 updater 内部写candidateRef.current。React 在 StrictMode 下会重复调用 updater，副作用会被执行多次。当前赋值是幂等的，所以现在没有可见缺陷。但这是反模式，后续修改容易引入真实 bug。建议在调用
setCandidate之前，先基于candidateRef.current计算并写入新值，再用同一个值更新 state。♻️ 建议改法
setIsGeocoding(false); setMessage(""); + const currentRef = candidateRef.current; + if ( + currentRef && + currentRef.longitude === longitude && + currentRef.latitude === latitude + ) { + candidateRef.current = { ...currentRef, address }; + } setCandidate((current) => { if ( !current || current.longitude !== longitude || current.latitude !== latitude ) { return current; } - const next = { ...current, address }; - candidateRef.current = next; - return next; + return { ...current, address }; });🤖 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 `@frontend/src/components/LocationConfirmationPicker.tsx` around lines 193 - 204, 将 LocationConfirmationPicker 中 setCandidate 的 updater 保持为纯函数，移除其中对 candidateRef.current 的写入；在调用 setCandidate 前基于 candidateRef.current 计算 next，先同步写入 candidateRef.current，再使用同一个 next 值更新 state，同时保留现有坐标校验和无变化时的行为。
🤖 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 `@deploy/preview/nginx.conf`:
- Around line 35-38: 更新 nginx 配置中围绕 $arg_jscode 的校验：通过检查 $args 判断请求是否显式包含 jscode
参数，并在该参数值为空时返回 400，同时保留缺少 jscode 参数时的正常转发行为，避免 rewrite 生成重复的 jscode 参数。
In `@frontend/src/components/LocationConfirmationPicker.test.tsx`:
- Around line 94-104: 修复测试间共享状态：在
frontend/src/components/LocationConfirmationPicker.test.tsx 的 94-104 行将 amap
初始化改为每次 beforeEach 都调用 createAmapMock，并删除 145-148 行和 166-168 行对 convertFrom
的手动保存与恢复；同时在 106-142 行的测试 setup 中清除 LocationConfirmationPicker.tsx 的模块级
amapPromise 缓存，使用 vi.resetModules() 配合动态导入，或导出仅供测试使用的缓存重置函数。
In `@frontend/src/components/LocationConfirmationPicker.tsx`:
- Around line 306-351: 修复 requestLocation
在选点面板已打开时重复定位导致地图状态失效的问题：获取新坐标后复用已有地图实例并更新标记、地图视野及地址解析；若无法复用，则先销毁旧实例再触发重新初始化，确保
isMapReady、candidate.address 和确认状态最终恢复。补充测试覆盖面板打开后再次点击“获取当前位置”，并断言“确认此位置”最终可用。
---
Nitpick comments:
In `@frontend/src/components/LocationConfirmationPicker.test.tsx`:
- Around line 236-239: 在 LocationConfirmationPicker 测试文件中提取统一 stub
navigator.geolocation 的辅助函数，替换五个测试中的重复 Object.defineProperty 配置，并在 afterEach
中保存后恢复原始 navigator.geolocation，确保测试间不共享被修改的全局状态。
In `@frontend/src/components/LocationConfirmationPicker.tsx`:
- Around line 193-204: 将 LocationConfirmationPicker 中 setCandidate 的 updater
保持为纯函数，移除其中对 candidateRef.current 的写入；在调用 setCandidate 前基于 candidateRef.current
计算 next，先同步写入 candidateRef.current，再使用同一个 next 值更新 state，同时保留现有坐标校验和无变化时的行为。
🪄 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: 84c26f86-a89b-4f6c-9bed-64f43ac844ed
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
.github/WORKFLOWS.md.github/workflows/pr-preview.yml.github/workflows/preview.yml.github/workflows/release.ymldeploy/preview/Dockerfile.frontenddeploy/preview/compose.local.ymldeploy/preview/deploy-local.shdeploy/preview/nginx.conffrontend/.env.examplefrontend/README.mdfrontend/package.jsonfrontend/src/components/LocationConfirmationPicker.test.tsxfrontend/src/components/LocationConfirmationPicker.tsxfrontend/src/pages/CaseWorkspacePage.tsxfrontend/src/pages/VolunteerWorkspacePage.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/release.yml
- frontend/src/pages/CaseWorkspacePage.tsx
- frontend/src/pages/VolunteerWorkspacePage.tsx
| if ($arg_jscode != "") { | ||
| return 400; | ||
| } | ||
| rewrite ^/_AMapService/(.*)$ /$1?$amap_forward_args break; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
According to Nginx documentation, does the $arg_namevariable have the same empty value when a query parameter is absent and when it is present with an empty value, such as?jscode=?
💡 Result:
In the Nginx configuration language, the $arg_name variable does not distinguish between an absent query parameter and a present-but-empty query parameter (e.g., ?name=); both render as an empty string [1]. Internally, however, Nginx maintains a distinction between these states [1]. While the standard Nginx configuration language cannot differentiate between them, it is possible to observe this difference using modules that provide access to the internal variable states, such as the Nginx Lua module [2][1]. In Lua, a missing argument evaluates to nil, whereas an empty string argument evaluates to an empty string "" [2][1]. Because Nginx's built-in $arg_ variables are computed on-the-fly by scanning the query string each time they are referenced, they do not have a fixed value container [3][4]. When a parameter is absent, the variable internally holds a "not found" state, which the Nginx configuration processor converts to an empty string for output or comparison purposes [2][1][3]. Consequently, simple configuration directives like if ($arg_name = "") will evaluate as true for both absent and empty parameters [1].
Citations:
- 1: https://www.getpagespeed.com/server-setup/nginx/nginx-variables
- 2: https://github.com/openresty/nginx-tutorials/blob/master/en/01-NginxVariables07.tut
- 3: https://openresty.org/download/agentzh-nginx-tutorials-en.html
- 4: https://frankindev.com/2020/01/21/nginx-built-in-variables/
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target configuration ---'
cat -n deploy/preview/nginx.conf | sed -n '1,90p'
printf '%s\n' '--- related jscode and amap_forward_args references ---'
rg -n -C 3 'jscode|amap_forward_args|AMapService' deploy
printf '%s\n' '--- repository files relevant to nginx configuration ---'
git ls-files deployRepository: AG-Angui/Angui
Length of output: 3644
🌐 Web query:
site:nginx.org/en/docs/http/ngx_http_core_module.html $arg_name empty query argument absent
💡 Result:
There is no standard technical definition or specific programming concept associated with the phrase "arg_name empty query argument absent." The provided search results did not return any authoritative documentation or industry standard definitions for this specific sequence of terms. In general software engineering and web development contexts, these terms refer to distinct states when handling request parameters: 1. Argument Absent: This occurs when a specific query parameter (often referred to as an argument) is entirely missing from the request URI (e.g., example.com/page does not include?arg_name=...). 2. Empty Argument: This occurs when the parameter is present in the URI but has no assigned value (e.g., example.com/page?arg_name=). Developers typically handle these cases by implementing validation logic to check if a parameter exists and if its value is null, undefined, or an empty string before processing the request.
Citations:
🌐 Web query:
site:nginx.org/en/docs/http/ngx_http_rewrite_module.html rewrite replacement question mark original arguments
💡 Result:
In the context of Nginx rewrite directives, placing a question mark at the end of a replacement string is used to prevent the original request arguments from being automatically appended to the rewritten URI [1]. By default, if a replacement string includes new request arguments, Nginx automatically appends the original request arguments to the end of the new URI [1]. Adding a question mark at the end of the replacement string signifies the end of the argument string, effectively instructing Nginx to discard or ignore the original request arguments [1]. For example, in the directive: rewrite ^/users/(.*)$ /show?user=$1? last; The question mark after $1 prevents the original request arguments from being appended to the /show?user=$1 URI [1].
Citations:
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from urllib.parse import parse_qsl
secret = "SECRET"
def nginx_arg(args, name):
# Model the documented nginx behavior relevant to this condition:
# an absent argument and an argument with an empty value both compare
# as an empty string through $arg_name.
for key, value in parse_qsl(args, keep_blank_values=True):
if key == name:
return value
return ""
def forwarded_args(args):
return f"jscode={secret}" if args == "" else f"{args}&jscode={secret}"
for args in ("", "foo=1", "jscode=", "foo=1&jscode=", "jscode=client&foo=1"):
rejected = nginx_arg(args, "jscode") != ""
rewritten = forwarded_args(args)
print(f"{args!r}: rejected={rejected}, rewritten_args={rewritten!r}")
PYRepository: AG-Angui/Angui
Length of output: 498
拒绝空的客户端 jscode 参数。
$arg_jscode 无法区分缺少参数和 ?jscode=。因此，空参数会通过检查，并生成两个 jscode 参数。请检查 $args 中是否存在 jscode，并在参数值为空时返回 400。
🤖 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 `@deploy/preview/nginx.conf` around lines 35 - 38, 更新 nginx 配置中围绕 $arg_jscode
的校验：通过检查 $args 判断请求是否显式包含 jscode 参数，并在该参数值为空时返回 400，同时保留缺少 jscode 参数时的正常转发行为，避免
rewrite 生成重复的 jscode 参数。
| beforeEach(() => { | ||
| amap ??= createAmapMock(); | ||
| amapLoad.mockReset(); | ||
| amapLoad.mockResolvedValue(amap.api as never); | ||
| vi.stubEnv("VITE_AMAP_JS_API_KEY", "test-key"); | ||
| vi.stubEnv("VITE_AMAP_JS_API_SERVICE_HOST", "/_AMapService"); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.unstubAllEnvs(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
测试之间存在共享状态，必须一起修复。 共同根因是状态的生命周期超出了单个测试：amap mock 对象在整个 describe 中只创建一次，LocationConfirmationPicker.tsx 中的模块级 amapPromise 也在测试之间保留。只修其中一处会让测试套件失败。
frontend/src/components/LocationConfirmationPicker.test.tsx#L94-L104：把amap ??= createAmapMock()改为amap = createAmapMock()，并删除第 145-148 行、第 166-168 行对convertFrom的手动保存与恢复。frontend/src/components/LocationConfirmationPicker.test.tsx#L106-L142：同时清除模块级amapPromise缓存。在beforeEach中调用vi.resetModules()并动态导入组件，或从组件模块导出一个仅测试使用的缓存重置函数。
📍 Affects 1 file
frontend/src/components/LocationConfirmationPicker.test.tsx#L94-L104(this comment)frontend/src/components/LocationConfirmationPicker.test.tsx#L106-L142
🤖 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 `@frontend/src/components/LocationConfirmationPicker.test.tsx` around lines 94
- 104, 修复测试间共享状态：在 frontend/src/components/LocationConfirmationPicker.test.tsx 的
94-104 行将 amap 初始化改为每次 beforeEach 都调用 createAmapMock，并删除 145-148 行和 166-168 行对
convertFrom 的手动保存与恢复；同时在 106-142 行的测试 setup 中清除 LocationConfirmationPicker.tsx
的模块级 amapPromise 缓存，使用 vi.resetModules() 配合动态导入，或导出仅供测试使用的缓存重置函数。
Preview readyThe preview for this build is available at https://pr-179-7759a81bec51.angui.cg8.site. Temporary demo accounts (all use the password
|
Preview readyThe preview for this build is available at https://pr-179-af36dc558280.angui.cg8.site. Temporary demo accounts (all use the password
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deploy/preview/nginx.conf (1)
34-38: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift限制公开 AMap 代理的路径和调用速率。
该路由会将任意
/_AMapService/<path>重写到对应的上游路径，并追加服务端jscode。调用者可以通过公开的浏览器端 key 访问未列入业务需求的restapi.amap.com接口，并消耗 AMap 配额。请仅放行
LocationConfirmationPicker所需的请求路径，并配置limit_req或等效限流。🤖 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 `@deploy/preview/nginx.conf` around lines 34 - 38, 限制 nginx 配置中 location ^~ /_AMapService/ 的代理范围，仅允许 LocationConfirmationPicker 所需的明确请求路径，拒绝其他 AMap 上游路径；同时为该路由配置现有 nginx 限流机制（limit_req 或等效方案），保留 GET-only 访问控制并确保请求仍能正常转发。Source: Path instructions
🧹 Nitpick comments (2)
deploy/preview/nginx.conf (1)
22-23: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win不要在整个
/api/路由关闭请求和响应缓冲。
proxy_buffering off和proxy_request_buffering off当前作用于所有 API。慢客户端会直接占用上游连接。SSE 通常只需要关闭响应缓冲。请将这些设置限制到实际的 SSE 路由，或确认所有/api/端点都需要流式传输。As per path instructions：该检查覆盖
deploy/**的容器可用性和路由回归。🤖 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 `@deploy/preview/nginx.conf` around lines 22 - 23, 将 proxy_buffering 和 proxy_request_buffering 的配置从整个 /api/ 路由范围移除，并仅应用到实际需要流式传输的 SSE 路由；保留普通 API 的请求和响应缓冲，除非配置中明确确认所有端点都需要流式传输。Source: Path instructions
frontend/src/api/client.test.ts (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议补充
error事件的测试用例。当前测试覆盖了
completed事件和不完整流(stream_incomplete)两种情况,但没有覆盖event: error分支(对应frontend/src/api/client.tsLine 188-197)。这是本次改动新增的、逻辑相对复杂的分支(状态码固定为 500、消息直接来自负载),建议补充一个测试用例,验证apiSseRequest在收到error事件时的行为符合预期。✅ 示例测试
+ it("rejects with the error event message when the stream reports a failure", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue( + new Response( + "event: error\ndata: {\"message\":\"session not found\"}\n\n", + { status: 201, headers: { "Content-Type": "text/event-stream" } }, + ), + ), + ); + + await expect( + apiSseRequest("/intake-sessions/intake-1/profile-draft/generate", { + method: "POST", + }, "test-session"), + ).rejects.toMatchObject({ code: "stream_failed", message: "session not found" }); + });Also applies to: 90-133
🤖 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 `@frontend/src/api/client.test.ts` at line 2, 在 apiSseRequest 的测试中补充 error 事件用例，模拟服务端发送 event: error 的 SSE 负载，并验证请求以状态码 500、错误消息直接取自负载的预期结果结束；保持现有 completed 和 stream_incomplete 测试不变。
🤖 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 `@frontend/src/api/client.ts`:
- Around line 151-157: Update userMessageFor to handle stream_missing,
invalid_stream, stream_interrupted, and stream_incomplete with dedicated
user-facing messages before the generic status === 0 fallback. Ensure each
status-0 stream error preserves its specific failure reason instead of
displaying the network-connection message.
In `@src/api/routes/intake_sessions.rs`:
- Around line 105-150: The SSE error contract must preserve structured status or
error-code information so the frontend can localize messages and branch on
specific statuses. In src/api/routes/intake_sessions.rs lines 105-150, update
the generate_intake_profile_draft error path and shared
sse_event/intake_sse_response flow to serialize fields derived from ApiError
instead of only error.to_string(); apply the same change to
start_ai_initial_review at lines 248-262. In frontend/src/api/client.ts lines
187-197, consume the new status/error-code fields, pass them through
userMessageFor, and retain specific handling for statuses such as 403, 404, and
409 rather than displaying payload.message directly.
- Around line 105-121: Update the SSE error path around
generate_intake_profile_draft and sse_event to serialize structured ApiError
information, including its error code and applicable HTTP status, instead of
error.to_string(). Update apiSseRequest to preserve that classification rather
than forcing stream_failed/500, and have FamilyIntakeForm map the code through
userMessageFor so all displayed messages are localized, including Internal and
Database errors.
In `@src/integrations/ai_gateway.rs`:
- Around line 859-866: The AI gateway diagnostic logs expose sensitive request
and response contents through native.body(), raw_response, and streamed chunk
logging. Update the relevant logging paths to remove complete payloads and
record only redacted/truncated data or safe metadata such as byte length, field
counts, or a content hash, covering both non-streaming and streaming responses
while preserving provider/model/protocol diagnostics.
---
Outside diff comments:
In `@deploy/preview/nginx.conf`:
- Around line 34-38: 限制 nginx 配置中 location ^~ /_AMapService/ 的代理范围，仅允许
LocationConfirmationPicker 所需的明确请求路径，拒绝其他 AMap 上游路径；同时为该路由配置现有 nginx
限流机制（limit_req 或等效方案），保留 GET-only 访问控制并确保请求仍能正常转发。
---
Nitpick comments:
In `@deploy/preview/nginx.conf`:
- Around line 22-23: 将 proxy_buffering 和 proxy_request_buffering 的配置从整个 /api/
路由范围移除，并仅应用到实际需要流式传输的 SSE 路由；保留普通 API 的请求和响应缓冲，除非配置中明确确认所有端点都需要流式传输。
In `@frontend/src/api/client.test.ts`:
- Line 2: 在 apiSseRequest 的测试中补充 error 事件用例，模拟服务端发送 event: error 的 SSE
负载，并验证请求以状态码 500、错误消息直接取自负载的预期结果结束；保持现有 completed 和 stream_incomplete 测试不变。
🪄 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: fa362aab-3889-4d05-a198-a0c0b6356e40
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
Cargo.tomldeploy/preview/nginx.conffrontend/src/api/client.test.tsfrontend/src/api/client.tsfrontend/src/api/intake.tsfrontend/src/pages/FamilyIntakeForm.tsxsrc/api/routes/intake_sessions.rssrc/integrations/ai_gateway.rs
| if (!response.body) { | ||
| throw new ApiClientError( | ||
| 0, | ||
| "stream_missing", | ||
| userMessageFor(0, "stream_missing"), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
新增的流错误码始终显示“网络连接失败”,掩盖真实原因。
userMessageFor 的第一条判断是 if (status === 0) return "网络连接失败,请检查服务连接后重试。";,该判断在检查 code 参数之前就返回。这里的四种新错误码——stream_missing(Line 151-157)、invalid_stream(Line 181-186)、stream_interrupted(Line 203-207)、stream_incomplete(Line 212-217)——全部以 status 0 调用 userMessageFor,因此都得到同一条网络错误提示。
这些失败原因并不相同。例如 invalid_stream 表示服务端返回了无法解析的 JSON,是服务端问题,不是网络问题。向用户提示“检查网络连接”会造成误导,也会让用户或客服无法根据提示区分真实故障。
建议在 userMessageFor 中为这些错误码补充专属分支,而不是依赖 status === 0 的通用回退。
As per path instructions for frontend/**: "Focus on frontend/backend API contract compatibility, authentication state, validation, data exposure, and user-visible failure paths."
💡 示例修复方向
function userMessageFor(
status: number,
code: string,
hasSession = false,
): string {
- if (status === 0) return "网络连接失败，请检查服务连接后重试。";
+ if (code === "stream_missing") return "服务未能返回结果流，请稍后重试。";
+ if (code === "invalid_stream") return "服务返回的数据格式异常，请稍后重试或联系支持。";
+ if (code === "stream_interrupted") return "连接在处理过程中中断，请稍后重试。";
+ if (code === "stream_incomplete") return "未能获取到完整结果，请稍后重试。";
+ if (status === 0) return "网络连接失败，请检查服务连接后重试。";
if (status === 400 || code === "validation_error")Also applies to: 181-186, 203-207, 212-217
🤖 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 `@frontend/src/api/client.ts` around lines 151 - 157, Update userMessageFor to
handle stream_missing, invalid_stream, stream_interrupted, and stream_incomplete
with dedicated user-facing messages before the generic status === 0 fallback.
Ensure each status-0 stream error preserves its specific failure reason instead
of displaying the network-connection message.
Source: Path instructions
| tokio::spawn(async move { | ||
| let event = match intake_session_service::generate_intake_profile_draft( | ||
| &state.db, | ||
| &auth, | ||
| &session_id, | ||
| &state.ai_gateway, | ||
| ) | ||
| .await?, | ||
| )) | ||
| .await | ||
| { | ||
| Ok(draft) => sse_event("completed", json!(draft)), | ||
| Err(error) => sse_event("error", json!({ "message": error.to_string() })), | ||
| }; | ||
| let _ = sender.send(event).await; | ||
| }); | ||
|
|
||
| Ok(intake_sse_response(receiver)) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect ApiError's Display/ResponseError implementation.
ast-grep run --pattern 'impl std::fmt::Display for ApiError { $$$ }' --lang rust src
ast-grep run --pattern 'impl ResponseError for ApiError { $$$ }' --lang rust src
rg -n -C5 'enum ApiError' srcRepository: AG-Angui/Angui
Length of output: 2553
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- src/api/error.rs ---'
cat -n src/api/error.rs
printf '%s\n' '--- intake_sessions.rs relevant sections ---'
sed -n '1,180p' src/api/routes/intake_sessions.rs | cat -n
printf '%s\n' '--- frontend apiSseRequest ---'
sed -n '150,215p' frontend/src/api/client.ts | cat -n
printf '%s\n' '--- FamilyIntakeForm error handling ---'
rg -n -C8 'apiSseRequest|cause\.status|stream_failed|status === 409|intake' frontend/src/pages/FamilyIntakeForm.tsx frontend/src/api/client.ts
printf '%s\n' '--- generate_intake_profile_draft call sites ---'
rg -n -C5 'generate_intake_profile_draft|intake_sse_response|sse_event' src frontendRepository: AG-Angui/Angui
Length of output: 36270
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- service function definitions ---'
sed -n '350,455p' src/application/services/intake_session_service.rs | cat -n
sed -n '200,275p' src/api/routes/intake_sessions.rs | cat -n
printf '%s\n' '--- ApiError constructions in intake service ---'
rg -n -C4 'ApiError::(Validation|NotFound|Conflict|Unauthorized|Forbidden|RateLimited|Internal|Database)|return Err|map_err' src/application/services/intake_session_service.rs
printf '%s\n' '--- frontend intake API definitions ---'
sed -n '1,260p' frontend/src/api/intake.ts | cat -n
printf '%s\n' '--- frontend SSE call sites ---'
rg -n -C8 'generateIntake|startIntakeAiInitialReview|apiSseRequest' frontend/srcRepository: AG-Angui/Angui
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- client error mapping ---'
sed -n '1,125p' frontend/src/api/client.ts | cat -n
printf '%s\n' '--- relevant intake API definitions ---'
rg -n -A12 -B3 'startIntakeAiInitialReview|getIntakeAiInitialReview|generateIntakeDraft' frontend/src/api/intake.ts
printf '%s\n' '--- exact page calls and nearby catches ---'
rg -n -A28 -B18 'generateIntakeDraft|startIntakeAiInitialReview' frontend/src/pages/FamilyIntakeForm.tsx
printf '%s\n' '--- page error-message helper and status branches ---'
rg -n -A18 -B8 'function messageFrom|const messageFrom|messageFrom\\(|cause\\.status' frontend/src/pages/FamilyIntakeForm.tsxRepository: AG-Angui/Angui
Length of output: 9524
🏁 Script executed:
#!/bin/bash
set -eu
rg -n -F -A20 -B8 'function messageFrom' frontend/src/pages/FamilyIntakeForm.tsx frontend/src
rg -n -F -A12 -B6 'messageFrom =' frontend/src/pages/FamilyIntakeForm.tsx frontend/srcRepository: AG-Angui/Angui
Length of output: 9062
为 SSE 错误保留结构化错误分类，并使用本地化消息。
两个 SSE 路由只发送 error.to_string()；apiSseRequest 随后固定生成 500/stream_failed，FamilyIntakeForm 直接展示该文本。这样会丢失 Forbidden、NotFound 和 Conflict 的状态及错误码。请发送 ApiError 的 code（必要时包含状态码），并在前端通过 userMessageFor 映射消息。当前 Internal 和 Database 文本虽已泛化，但字符串变体仍会透传未本地化的技术文本。
🤖 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 `@src/api/routes/intake_sessions.rs` around lines 105 - 121, Update the SSE
error path around generate_intake_profile_draft and sse_event to serialize
structured ApiError information, including its error code and applicable HTTP
status, instead of error.to_string(). Update apiSseRequest to preserve that
classification rather than forcing stream_failed/500, and have FamilyIntakeForm
map the code through userMessageFor so all displayed messages are localized,
including Internal and Database errors.
Source: Path instructions
| tokio::spawn(async move { | ||
| let event = match intake_session_service::generate_intake_profile_draft( | ||
| &state.db, | ||
| &auth, | ||
| &session_id, | ||
| &state.ai_gateway, | ||
| ) | ||
| .await?, | ||
| )) | ||
| .await | ||
| { | ||
| Ok(draft) => sse_event("completed", json!(draft)), | ||
| Err(error) => sse_event("error", json!({ "message": error.to_string() })), | ||
| }; | ||
| let _ = sender.send(event).await; | ||
| }); | ||
|
|
||
| Ok(intake_sse_response(receiver)) | ||
| } | ||
|
|
||
| fn intake_sse_response(receiver: mpsc::Receiver<web::Bytes>) -> HttpResponse { | ||
| let events = stream::unfold( | ||
| (receiver, tokio::time::interval(Duration::from_secs(10))), | ||
| |(mut receiver, mut heartbeat)| async move { | ||
| tokio::select! { | ||
| event = receiver.recv() => event.map(|event| { | ||
| (Ok::<_, Error>(event), (receiver, heartbeat)) | ||
| }), | ||
| _ = heartbeat.tick() => Some(( | ||
| Ok(web::Bytes::from_static(b": keep-alive\n\n")), | ||
| (receiver, heartbeat), | ||
| )), | ||
| } | ||
| }, | ||
| ); | ||
|
|
||
| HttpResponse::Created() | ||
| .insert_header((header::CONTENT_TYPE, "text/event-stream")) | ||
| .insert_header((header::CACHE_CONTROL, "no-cache")) | ||
| .insert_header(("X-Accel-Buffering", "no")) | ||
| .streaming(events) | ||
| } | ||
|
|
||
| fn sse_event(event: &str, payload: serde_json::Value) -> web::Bytes { | ||
| let payload = serde_json::to_string(&payload) | ||
| .unwrap_or_else(|_| "{\"message\":\"internal service error\"}".to_owned()); | ||
| web::Bytes::from(format!("event: {event}\ndata: {payload}\n\n")) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
SSE error 事件契约丢失状态分类,前端始终显示未本地化的原始文本。 根因在后端:error 事件只携带 error.to_string(),不携带任何状态或错误码;前端据此始终返回状态 500、直接透传该文本作为用户提示。
src/api/routes/intake_sessions.rs#L105-L150:在generate_intake_profile_draft的错误分支和共享的sse_event/intake_sse_response辅助函数中,补充结构化的状态或错误码字段(例如从ApiError派生),而不是只序列化error.to_string()。src/api/routes/intake_sessions.rs#L248-L262:start_ai_initial_review使用了相同的模式,需要同步应用上述修复。frontend/src/api/client.ts#L187-L197:改用后端新增的状态/错误码字段,通过userMessageFor重新映射为本地化提示文本,而不是直接使用payload.message,并恢复对具体状态码(如 403/404/409)的分支能力。
📍 Affects 2 files
src/api/routes/intake_sessions.rs#L105-L150(this comment)src/api/routes/intake_sessions.rs#L248-L262frontend/src/api/client.ts#L187-L197
🤖 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 `@src/api/routes/intake_sessions.rs` around lines 105 - 150, The SSE error
contract must preserve structured status or error-code information so the
frontend can localize messages and branch on specific statuses. In
src/api/routes/intake_sessions.rs lines 105-150, update the
generate_intake_profile_draft error path and shared
sse_event/intake_sse_response flow to serialize fields derived from ApiError
instead of only error.to_string(); apply the same change to
start_ai_initial_review at lines 248-262. In frontend/src/api/client.ts lines
187-197, consume the new status/error-code fields, pass them through
userMessageFor, and retain specific handling for statuses such as 403, 404, and
409 rather than displaying payload.message directly.
| log::debug!( | ||
| target: "angui::ai_gateway::diagnostic", | ||
| "AI provider request: provider_id={}, model={}, protocol={:?}, body={}", | ||
| route.provider_id, | ||
| route.model, | ||
| route.protocol, | ||
| native.body(), | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
诊断日志记录了完整的 AI 请求体与响应体,存在 PII 泄露风险。
- Line 859-866:记录
native.body(),即发送给 AI 供应商的完整请求体。该请求体包含家属提交的走失人员体貌、衣着、健康注意事项等描述性个人信息。 - Line 897-909:非成功响应时记录供应商返回的原始文本
raw_response。 - Line 925-940:非流式成功响应时,同样记录完整的
raw_response,其中包含 AI 生成的走失人员画像草稿。 - Line 1011、1025-1031:流式响应场景下,记录每一个原始 SSE 分块
String::from_utf8_lossy(&chunk),同样可能包含部分画像内容。
这些内容都属于 log 的调试级别,只有在该 target 的调试日志被启用时才会被记录。但一旦生产环境为了排查问题临时调高日志级别,家属提交的敏感个人信息(可能包含健康相关内容)就会被完整写入应用日志。系统已经通过 AiRequest.redaction_policy_version 体现出对这类数据的治理意识,这里的诊断日志绕开了该治理机制,把同样的数据额外复制进访问范围更广、留存时间通常更长的应用日志中。
建议在记录前对请求体/响应体做脱敏或截断处理,或只记录摘要信息(如字段数量、字节长度、内容哈希),避免记录完整原始内容。
As per path instructions for src/**: "Review Rust/Actix code for authentication, authorization, privacy, error handling, concurrency, API compatibility, and data-loss risks."
Also applies to: 897-909, 925-940, 1011-1011, 1025-1031
🤖 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 `@src/integrations/ai_gateway.rs` around lines 859 - 866, The AI gateway
diagnostic logs expose sensitive request and response contents through
native.body(), raw_response, and streamed chunk logging. Update the relevant
logging paths to remove complete payloads and record only redacted/truncated
data or safe metadata such as byte length, field counts, or a content hash,
covering both non-streaming and streaming responses while preserving
provider/model/protocol diagnostics.
Source: Path instructions
Preview readyThe preview for this build is available at https://pr-179-ec9d3ede2283.angui.cg8.site. Temporary demo accounts (all use the password
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/api/intake_session_confirm_post.rs`:
- Around line 175-182: 扩展 intake session SSE 测试覆盖：在现有成功场景中断言 started.session_id
等于 session_id，并保留 completed 数据断言；新增 start_ai_initial_review 返回 Err 时的测试，断言 SSE
error.message，避免使用 AiExecutionResult::Failed（其会走规则回退）；补充仅含 started、缺少 completed
以及多段 data: 的边界测试，并按预期验证解析失败或 data 合并行为。
🪄 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: b36c46fd-8927-43e9-9889-dcf81c309fa1
📒 Files selected for processing (2)
tests/api/intake_session_confirm_post.rstests/support/mod.rs
| assert_eq!( | ||
| initial_review | ||
| .headers() | ||
| .get(header::CONTENT_TYPE) | ||
| .and_then(|value| value.to_str().ok()), | ||
| Some("text/event-stream") | ||
| ); | ||
| let initial_body = read_sse_completed_json(initial_review).await; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'read_sse_completed_json|read_sse_.*error|sse_event\("(started|completed|error)"|event: (started|completed|error)' \
tests srcRepository: AG-Angui/Angui
Length of output: 9376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- route structure and SSE producer ---'
ast-grep outline src/api/routes/intake_sessions.rs
sed -n '80,135p' src/api/routes/intake_sessions.rs
printf '%s\n' '--- SSE helper and related tests ---'
sed -n '185,230p' tests/support/mod.rs
sed -n '130,205p' tests/api/intake_session_confirm_post.rs
sed -n '270,390p' tests/api/intake_session_confirm_post.rs
printf '%s\n' '--- all relevant event construction and parser usage ---'
rg -n -C 5 'fn sse_event|sse_event\(|read_sse_' src testsRepository: AG-Angui/Angui
Length of output: 20174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- initial-review route and service call graph ---'
sed -n '225,267p' src/api/routes/intake_sessions.rs
rg -n -C 8 'generate_intake_initial_review|StartIntakeAiInitialReviewRequest|ai_gateway|Mock.*Gateway|failure|error' src tests
printf '%s\n' '--- test support and gateway setup ---'
rg -n -C 8 'struct TestContext|impl TestContext|app_state|AiGateway|ai_gateway|mock' tests/support testsRepository: AG-Angui/Angui
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact initial-review implementation and gateway behavior ---'
rg -n '^(pub )?async fn (start_ai_initial_review|generate_intake_profile_draft)|generate_intake_profile_draft|from_configurations|struct TestContext|fn new\(' src/application src/ai_gateway.rs tests/support/mod.rs
sed -n '1,95p' tests/support/mod.rs
sed -n '1,220p' src/ai_gateway.rsRepository: AG-Angui/Angui
Length of output: 668
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gateway_file="$(git ls-files | rg '(^|/)ai_gateway\.rs$' | head -n 1)"
printf 'gateway_file=%s\n' "$gateway_file"
sed -n '25,65p' tests/support/mod.rs
sed -n '360,510p' src/application/services/intake_session_service.rs
if [ -n "$gateway_file" ]; then
sed -n '1,240p' "$gateway_file"
fiRepository: AG-Angui/Angui
Length of output: 17094
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- remaining initial-review service logic ---'
sed -n '500,590p' src/application/services/intake_session_service.rs
printf '%s\n' '--- gateway construction, routing, and execution ---'
rg -n -C 10 'impl AiGateway|from_configurations|pub async fn execute|AiExecutionResult::Failed|AiExecutionResult::Degraded|NoProviderConfigured' src/integrations/ai_gateway.rsRepository: AG-Angui/Angui
Length of output: 30845
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
def read_sse_completed_json(stream):
for frame in stream.split("\n\n"):
event = next(
(line.removeprefix("event:").strip()
for line in frame.splitlines()
if line.startswith("event:")),
None,
)
if event != "completed":
continue
payload = "\n".join(
line.removeprefix("data:").lstrip()
for line in frame.splitlines()
if line.startswith("data:")
)
return json.loads(payload)
raise RuntimeError("SSE response should contain a completed event")
cases = {
"started_only": "event: started\ndata: {\"session_id\":\"s\"}\n\n",
"completed_missing": "event: started\ndata: {\"session_id\":\"s\"}\n\n",
"completed_multiple_data": (
"event: completed\n"
"data: {\"status\":\n"
"data: \"awaiting_family_review\"}\n\n"
),
}
for name, stream in cases.items():
try:
value = read_sse_completed_json(stream)
print(name, "=>", value)
except Exception as exc:
print(name, "=>", type(exc).__name__, str(exc))
PYRepository: AG-Angui/Angui
Length of output: 371
补充 SSE 事件的正向、负向和边界覆盖。
当前测试只断言 completed。补充以下断言：
- 正向：断言
started.session_id等于session_id，并保留completed数据断言。 - 负向：使
start_ai_initial_review返回Err，然后断言error.message。AiExecutionResult::Failed当前会转换为规则回退，不会触发 SSEerror。 - 边界：覆盖仅有
started、缺少completed和多段data:的响应。当前解析器对前两种情况失败，并会合并多段data:。
🤖 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 `@tests/api/intake_session_confirm_post.rs` around lines 175 - 182, 扩展 intake
session SSE 测试覆盖：在现有成功场景中断言 started.session_id 等于 session_id，并保留 completed
数据断言；新增 start_ai_initial_review 返回 Err 时的测试，断言 SSE error.message，避免使用
AiExecutionResult::Failed（其会走规则回退）；补充仅含 started、缺少 completed 以及多段 data:
的边界测试，并按预期验证解析失败或 data 合并行为。
Source: Path instructions
Preview readyThe preview for this build is available at https://pr-179-ec9d3ede2283.angui.cg8.site. Temporary demo accounts (all use the password
|
Preview readyThe preview for this build is available at https://pr-179-ec9d3ede2283.angui.cg8.site. Temporary demo accounts (all use the password
|
- Support and normalize provider-specific profile_candidates output - Require fixed nullable source citations for every profile field - Validate excerpts against cited family answers with diagnostics - Add compatibility, provenance, prompt, and schema tests - Load local environment files before application initialization
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/application/services/intake_session_service.rs (1)
2226-2288: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议补充两条测试用例。
当前测试覆盖兼容形状与摘录不可验证的拒绝。以下两条路径未覆盖：
- canonical 形状解码：确认
decode_profile_extraction_output返回used_provider_compatibility_shape == false。- 跨字段来源冲突：
name与gender引用不同source_answer_field时，combine_profile_candidates必须返回physical_description candidates must cite the same family answer field。第二条是新增的强约束，回退行为依赖它。
🤖 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 `@src/application/services/intake_session_service.rs` around lines 2226 - 2288, 补充两条测试覆盖未验证路径：在 profile_extraction 测试中使用 canonical 输出调用 decode_profile_extraction_output，并断言 used_provider_compatibility_shape 为 false；另增加跨字段来源冲突测试，让 name 与 gender 引用不同的 source_answer_field，调用 combine_profile_candidates，并断言返回 “physical_description candidates must cite the same family answer field”。
🤖 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.
Nitpick comments:
In `@src/application/services/intake_session_service.rs`:
- Around line 2226-2288: 补充两条测试覆盖未验证路径：在 profile_extraction 测试中使用 canonical 输出调用
decode_profile_extraction_output，并断言 used_provider_compatibility_shape 为
false；另增加跨字段来源冲突测试，让 name 与 gender 引用不同的 source_answer_field，调用
combine_profile_candidates，并断言返回 “physical_description candidates must cite the
same family answer field”。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 06e8d709-0d64-4507-816b-2f60d04f72eb
📒 Files selected for processing (2)
src/application/services/intake_session_service.rssrc/main.rs
Preview readyThe preview for this build is available at https://pr-179-c1256b3d1493.angui.cg8.site. Temporary demo accounts (all use the password
|
…ng fields - Set `Content-Type: application/json` for streamed requests with non-FormData bodies - Preserve existing headers and authorization while setting SSE acceptance headers - Allow initial review issues to reference configured fields that remain unanswered - Restrict review fields and source fields to the configured intake answer fields - Log rejected AI review output before falling back to rule-based issues - Add coverage for streamed JSON headers and initial review validation behavior
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/api/client.ts (1)
195-203: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win统一 SSE 错误响应契约并保留错误码。
后端发送顶层
{ "message": ... }，而 JSON API 使用{ "error": { "code", "message" } }。客户端将所有 SSE 错误转换为500/stream_failed，因此会丢失unauthorized、forbidden、not_found和conflict，且无法触发会话过期处理。为 SSE 错误增加稳定的code和状态信息，并在客户端使用userMessageFor映射用户消息，不要直接显示服务端message。🤖 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 `@frontend/src/api/client.ts` around lines 195 - 203, 更新 SSE 的 error 处理分支，使其按 JSON API 契约读取顶层 error 对象中的 code 和状态信息，而不是仅读取顶层 message。保留后端返回的 unauthorized、forbidden、not_found、conflict 等错误码及对应状态创建 ApiClientError，并通过 userMessageFor 映射用户提示，避免直接展示服务端 message；同时兼容统一后的 SSE 错误响应结构。Source: Path instructions
🧹 Nitpick comments (1)
frontend/src/api/client.test.ts (1)
90-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win补充 SSE 失败路径的契约测试。
当前新增测试覆盖
completed、keep-alive、请求头和stream_incomplete。未看到error事件、无效 JSON 和reader.read()失败的测试。请至少补充这些场景，并断言ApiClientError.code，以锁定用户可见的错误处理契约。As per path instructions: 对
frontend/**，重点检查 frontend/backend API contract compatibility、authentication state、validation、data exposure 和 user-visible failure paths。🤖 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 `@frontend/src/api/client.test.ts` around lines 90 - 155, 在 apiSseRequest 测试组中补充 SSE 失败路径契约测试：覆盖 error 事件、data 无效 JSON，以及 reader.read() 失败，并分别断言抛出的 ApiClientError.code 为对应的用户可见错误码。保持现有 completed、keep-alive、请求头和 stream_incomplete 测试不变。Source: Path instructions
🤖 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.
Outside diff comments:
In `@frontend/src/api/client.ts`:
- Around line 195-203: 更新 SSE 的 error 处理分支，使其按 JSON API 契约读取顶层 error 对象中的 code
和状态信息，而不是仅读取顶层 message。保留后端返回的 unauthorized、forbidden、not_found、conflict
等错误码及对应状态创建 ApiClientError，并通过 userMessageFor 映射用户提示，避免直接展示服务端 message；同时兼容统一后的
SSE 错误响应结构。
---
Nitpick comments:
In `@frontend/src/api/client.test.ts`:
- Around line 90-155: 在 apiSseRequest 测试组中补充 SSE 失败路径契约测试：覆盖 error 事件、data 无效
JSON，以及 reader.read() 失败，并分别断言抛出的 ApiClientError.code 为对应的用户可见错误码。保持现有
completed、keep-alive、请求头和 stream_incomplete 测试不变。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: afe8a637-44ea-4207-97e7-0cd20db0674c
📒 Files selected for processing (3)
frontend/src/api/client.test.tsfrontend/src/api/client.tssrc/application/services/intake_session_service.rs
- load AMap PlaceSearch alongside geocoding services - search addresses, landmarks, and venues before opening map confirmation - show selectable POI results and center the picker on selection - handle empty queries, unavailable services, and unlocatable results - reset search state when clearing a confirmed location - move clue location picker beside clue content in the workspace form - add coverage for searching and selecting a location result
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@frontend/src/components/LocationConfirmationPicker.tsx`:
- Around line 425-435: 使 LocationConfirmationPicker 中的搜索结果始终对应当前关键词：在
searchPlaces 处理空关键词时先使 searchOperationRef 递增并清空 searchResults，再显示提示并返回；在更新
searchQuery 的处理逻辑中同步使正在执行的搜索失效并清空旧结果。保留现有请求回调的操作编号校验，确保关键词修改或提交空关键词后旧响应不会重新显示结果。
- Around line 263-280: 在 openPickerAtCoordinates 中，当 map.current
不存在且用户重新选择结果时递增地图初始化版本状态，并将该状态加入负责创建地图的 useEffect
依赖，使失效后的初始化重新执行；保持已有地图实例时的行为不变，并补充地图加载期间连续选择两个搜索结果仍能完成初始化的测试。
🪄 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: 05a9215d-25f1-480b-a44d-8d9af2ab4cd2
📒 Files selected for processing (3)
frontend/src/components/LocationConfirmationPicker.test.tsxfrontend/src/components/LocationConfirmationPicker.tsxfrontend/src/pages/CaseWorkspacePage.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/pages/CaseWorkspacePage.tsx
- frontend/src/components/LocationConfirmationPicker.test.tsx
| const openPickerAtCoordinates = useCallback( | ||
| (longitude: number, latitude: number) => { | ||
| ++operationRef.current; | ||
| const next: CandidateLocation = { | ||
| longitude, | ||
| latitude, | ||
| address: "", | ||
| precision: "approximate", | ||
| }; | ||
| candidateRef.current = next; | ||
| setCandidate(next); | ||
| setIsPickerOpen(true); | ||
| setMessage(""); | ||
| setSearchResults([]); | ||
| marker.current?.setPosition([longitude, latitude]); | ||
| map.current?.panTo([longitude, latitude]); | ||
| if (map.current) resolveAddress(longitude, latitude); | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
重新选择搜索结果时，重新启动地图初始化。
当首次地图初始化仍在等待时，用户可以再次选择搜索结果。Line 265 会使现有初始化失效。此时 isPickerOpen 仍为 true，map.current 仍为 null，useEffect 不会重新执行。地图不会创建，canConfirm 会一直为 false。
当 map.current 不存在时，增加一个地图初始化版本状态并将其加入 effect 依赖。每次选择新结果时递增该版本。补充“地图加载期间连续选择两个结果”的测试。
建议修复
+ const [mapInitializationRevision, setMapInitializationRevision] = useState(0);
+
const openPickerAtCoordinates = useCallback(
(longitude: number, latitude: number) => {
// ...
marker.current?.setPosition([longitude, latitude]);
map.current?.panTo([longitude, latitude]);
- if (map.current) resolveAddress(longitude, latitude);
+ if (map.current) {
+ resolveAddress(longitude, latitude);
+ } else {
+ setMapInitializationRevision((revision) => revision + 1);
+ }
},
[resolveAddress],
);
- }, [chooseCoordinates, isPickerOpen, resolveAddress]);
+ }, [chooseCoordinates, isPickerOpen, mapInitializationRevision, resolveAddress]);As per path instructions，需检查 frontend/** 的用户可见失败路径。
📝 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.
| const openPickerAtCoordinates = useCallback( | |
| (longitude: number, latitude: number) => { | |
| ++operationRef.current; | |
| const next: CandidateLocation = { | |
| longitude, | |
| latitude, | |
| address: "", | |
| precision: "approximate", | |
| }; | |
| candidateRef.current = next; | |
| setCandidate(next); | |
| setIsPickerOpen(true); | |
| setMessage(""); | |
| setSearchResults([]); | |
| marker.current?.setPosition([longitude, latitude]); | |
| map.current?.panTo([longitude, latitude]); | |
| if (map.current) resolveAddress(longitude, latitude); | |
| }, | |
| const [mapInitializationRevision, setMapInitializationRevision] = useState(0); | |
| const openPickerAtCoordinates = useCallback( | |
| (longitude: number, latitude: number) => { | |
| +operationRef.current; | |
| const next: CandidateLocation = { | |
| longitude, | |
| latitude, | |
| address: "", | |
| precision: "approximate", | |
| }; | |
| candidateRef.current = next; | |
| setCandidate(next); | |
| setIsPickerOpen(true); | |
| setMessage(""); | |
| setSearchResults([]); | |
| marker.current?.setPosition([longitude, latitude]); | |
| map.current?.panTo([longitude, latitude]); | |
| if (map.current) { | |
| resolveAddress(longitude, latitude); | |
| } else { | |
| setMapInitializationRevision((revision) => revision + 1); | |
| } | |
| }, |
🤖 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 `@frontend/src/components/LocationConfirmationPicker.tsx` around lines 263 -
280, 在 openPickerAtCoordinates 中，当 map.current
不存在且用户重新选择结果时递增地图初始化版本状态，并将该状态加入负责创建地图的 useEffect
依赖，使失效后的初始化重新执行；保持已有地图实例时的行为不变，并补充地图加载期间连续选择两个搜索结果仍能完成初始化的测试。
Source: Path instructions
- invalidate pending place searches when the input value changes - clear displayed results and loading state for changed or empty queries - show empty-query validation without retaining previous search results - add coverage for stale asynchronous search responses
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/components/LocationConfirmationPicker.test.tsx (1)
330-336: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win补充已显示结果在清空关键词后被移除的断言。
此处清空关键词前没有已显示的搜索结果。当前断言不能证明清空操作会移除最新搜索结果。若组件保留最新结果，此测试仍会通过。
先完成“新关键词”的搜索并断言结果显示。然后清空输入，并断言该结果已消失。
建议的测试补充
+ fireEvent.click(screen.getByRole("button", { name: "搜索地点" })); + await act(async () => { + pendingSearch?.("complete", { + poiList: { + pois: [{ + id: "current-place", + name: "新搜索结果", + location: { getLng: () => 116.41, getLat: () => 39.93 }, + }], + }, + }); + }); + expect(screen.getByText("新搜索结果")).toBeInTheDocument(); + fireEvent.change(screen.getByRole("textbox", { name: "搜索地点" }), { target: { value: "" }, }); + expect(screen.queryByText("新搜索结果")).not.toBeInTheDocument();🤖 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 `@frontend/src/components/LocationConfirmationPicker.test.tsx` around lines 330 - 336, 更新 LocationConfirmationPicker 测试中的搜索流程：先输入新的关键词并点击“搜索地点”，断言对应搜索结果已显示；随后清空输入并再次搜索，断言该结果已从文档中移除，同时保留现有的空关键词提示断言。
🤖 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.
Nitpick comments:
In `@frontend/src/components/LocationConfirmationPicker.test.tsx`:
- Around line 330-336: 更新 LocationConfirmationPicker
测试中的搜索流程：先输入新的关键词并点击“搜索地点”，断言对应搜索结果已显示；随后清空输入并再次搜索，断言该结果已从文档中移除，同时保留现有的空关键词提示断言。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b0dce574-ccf0-43a7-9936-8473c250a256
📒 Files selected for processing (2)
frontend/src/components/LocationConfirmationPicker.test.tsxfrontend/src/components/LocationConfirmationPicker.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/LocationConfirmationPicker.tsx
- Force PostCSS to use version 8.5.23 through Yarn resolutions - Upgrade Undici to version 7.29.0 for all matching dependency ranges - Update the lockfile with the new Undici package metadata and integrity hash
Preview readyThe preview for this build is available at https://pr-179-445aa2044826.angui.cg8.site. Temporary demo accounts (all use the password
|
Implements explicit browser geolocation and AMap JSAPI confirmation for supplemental places, supplemental clues, and volunteer field observations. Users can click or drag the map marker and only the final confirmed values update the existing forms. Manual entry and the existing review, visibility, and API contracts are preserved when permission or map loading fails. The browser key is separate from the server Web Service key and is injected only during frontend builds. Verification: the new picker tests, lint, TypeScript checking, and production build pass; the full frontend suite has one unrelated pre-existing FamilyIntakeForm test failure. Closes #169. Closes #170.
Summary by CodeRabbit
新功能
文档