[feat][backend] code evaluator #220
Conversation
(LogID: 202510101913360100911101348473F04) Co-Authored-By: Coda <[email protected]>
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #220 +/- ##
==========================================
+ Coverage 64.08% 64.30% +0.22%
==========================================
Files 477 481 +4
Lines 51830 52371 +541
==========================================
+ Hits 33214 33677 +463
- Misses 16286 16354 +68
- Partials 2330 2340 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
(LogID: 202510111803140100911101340639242) Co-Authored-By: Coda <[email protected]>
…rectory - Enhanced runtime package test coverage from 41.3% to 86.1% - Added comprehensive tests for RuntimeFactory, RuntimeManager, HTTPFaaSRuntimeAdapter - Extended PythonRuntime and JavaScriptRuntime testing with return_val functions - Added concurrent access testing and error handling verification - Implemented mock-based testing patterns following existing code style
- 移除测试文件中的多余空行 - 修复代码缩进不一致问题 - 使用go fmt格式化代码 这些修改仅涉及格式问题，未更改任何业务逻辑。
- 格式化运行时相关文件 - 修复代码缩进和空白字符问题 - 保持业务逻辑不变
- Fixed unchecked error returns in http_faas_runtime.go - Added safe env var helper functions in runtime_test.go - Removed unnecessary type conversions in evaluator.go - All packages now pass golangci-lint checks
…nto feat/open-code-eval
- Added missing Apache-2.0 license headers to 53 frontend TypeScript/TSX files - Fixed 48 files automatically using license-eye tool - Manually added headers to 5 .less files that weren't auto-fixable - All files now pass license header validation
This reverts commit 2cee2c1.
- 统一docker和k8s版本的响应格式 - 实现Pyodide预加载机制，使用vendor离线依赖 - 优化进程池管理，支持预创建进程 - 改进代码执行逻辑，使用临时文件避免转义问题 - 添加完整的错误处理和日志记录 - 支持return_val函数和多种返回值格式提取 - 删除ModelConfig配置
tpfz
force-pushed
the
feat/open-code-eval
branch
from
777a25a to
4879e6e
Compare
| # 后台健康检查循环 | ||
| ( | ||
| while true; do | ||
| if sh /coze-loop/bootstrap/js-faas/healthcheck.sh; then |
There was a problem hiding this comment.
/coze-loop/bootstrap/js-faas/healthcheck.sh 这个路径规范和其他组件的不一样，修改为保持一致吧；目录名在 docker-compose.yml 中定义的
| )& | ||
|
|
||
| # 启动JavaScript FaaS服务器 | ||
| exec deno run --allow-net=0.0.0.0:8000 --allow-env --allow-read=/coze-loop/bootstrap/js-faas,/tmp --allow-write=/tmp --allow-run /coze-loop/bootstrap/js-faas/js_faas_server.ts No newline at end of file |
There was a problem hiding this comment.
app 的启动时间会明显变长吗，为什么要调整这个检测次数
| EXPOSE 8000 | ||
|
|
||
| # 默认命令 - 使用 entrypoint 脚本 | ||
| CMD ["/app/entrypoint.sh"] |
| CMD deno eval "try { const resp = await fetch('http://localhost:8000/health'); if (resp.ok) { Deno.exit(0); } else { Deno.exit(1); } } catch (e) { Deno.exit(1); }" | ||
|
|
||
| # 暴露端口 | ||
| EXPOSE 8000 |
There was a problem hiding this comment.
在 docker-compose.yaml 中的 comment 对这个问题吧
| working_dir: /app | ||
| entrypoint: [ "sh", "/coze-loop/bootstrap/python-faas/entrypoint.sh" ] | ||
| deploy: | ||
| resources: |
| - SETGID | ||
| healthcheck: | ||
| test: [ "CMD", "sh", "/coze-loop/bootstrap/python-faas/healthcheck.sh" ] | ||
| interval: 60s |
There was a problem hiding this comment.
感觉 interval、timeout、start_period 设置时间太长了吧
| restart: always | ||
| networks: | ||
| - coze-loop-network | ||
| ports: |
| - "${COZE_LOOP_JS_FAAS_PORT:-8891}:8000" | ||
| volumes: | ||
| - js_faas_workspace:/tmp/faas-workspace | ||
| - ./bootstrap/js-faas:/coze-loop/bootstrap/js-faas |
| cpus: "${JS_FAAS_CPU_RESERVE:-0.25}" | ||
| healthcheck: | ||
| test: [ "CMD", "sh", "/coze-loop/bootstrap/js-faas/healthcheck.sh" ] | ||
| interval: 60s |
There was a problem hiding this comment.
Summary:
- Must-Fix: JS return_val is not captured by the current JS FaaS wrapper because it logs a raw string. Please emit a single JSON line (e.g.,
console.log(JSON.stringify({ret_val}))) so the server can parse it, or add an explicit marker protocol. - Should-Fix: Provide fail-open defaults for COZE_LOOP_JS_FAAS_URL and COZE_LOOP_PYTHON_FAAS_URL to improve robustness across dev/test environments.
- Suggestion: Use
normalizeLanguage(language)in the HTTP adapter, and include TypeScript aliases (ts/typescript) in config key normalization.
These changes will restore functional correctness for JS evaluators, reduce configuration friction, and make the runtime adapter more resilient.
| // return_val函数实现 | ||
| function return_val(value) { | ||
| /** | ||
| * 修复后的return_val函数实现 - 只输出ret_val内容 | ||
| * @param {string} value - 要返回的值，通常是JSON字符串 | ||
| */ | ||
| // 处理输入值 | ||
| const ret_val = (value === null || value === undefined) ? "" : String(value); | ||
| // 直接输出ret_val内容，供JavaScript FaaS服务器捕获 | ||
| console.log(ret_val); | ||
| } | ||
| ` |
There was a problem hiding this comment.
[🚫Must-Fix] JavaScript return_val not captured by FaaS wrapper
Problem: In the JS branch of GetReturnValFunction, the implementation logs a raw string with console.log(ret_val);. The JS FaaS server (js_faas_server.ts) extracts ret_val by parsing the last JSON line printed by user code. A plain string will not be recognized, causing ret_val to be empty in responses.
Why it matters: This breaks functional correctness for JS evaluators—downstream components expecting ret_val will receive empty values. It also creates inconsistency with the Python branch which emits a structured output with markers.
Fix: Emit a compact JSON object on a single line so the server reliably parses it. Example:
function return_val(value) {
const ret_val = (value === null || value === undefined) ? "" : String(value);
// Emit a single JSON line for the FaaS wrapper to parse
console.log(JSON.stringify({ ret_val }));
}Alternatively, align with the Python approach and add explicit start/end markers if the server supports them.
| pythonFaaSURL := os.Getenv("COZE_LOOP_PYTHON_FAAS_URL") | ||
| if pythonFaaSURL == "" { | ||
| return nil, fmt.Errorf("必须配置Python FaaS服务URL，请设置COZE_LOOP_PYTHON_FAAS_URL环境变量") | ||
| } |
There was a problem hiding this comment.
[🚨Should-Fix] Fail-open to default Python FaaS URL when env var is missing
Problem: NewPythonRuntime returns an error if COZE_LOOP_PYTHON_FAAS_URL is not set. This forces environment configuration in all contexts and can block evaluation in dev/test clusters that already expose the default service endpoint.
Fix: Provide a default URL fallback consistent with the adapter’s default. Example:
pythonFaaSURL := os.Getenv("COZE_LOOP_PYTHON_FAAS_URL")
if pythonFaaSURL == "" {
pythonFaaSURL = "http://coze-loop-python-faas:8000"
logger.WithField("python_faas_url", pythonFaaSURL).Warn("COZE_LOOP_PYTHON_FAAS_URL not set; using default")
}Benefit: Improves robustness and reduces configuration friction across environments.
| normalizedKey := langKey | ||
| switch strings.ToLower(langKey) { | ||
| case "python": | ||
| normalizedKey = string(evaluatordto.LanguageTypePython) | ||
| case "js", "javascript": | ||
| normalizedKey = string(evaluatordto.LanguageTypeJS) | ||
| } |
There was a problem hiding this comment.
[💡Suggestion] Normalize language keys to include TypeScript aliases
Observation: When standardizing the second-level language keys, JS normalization only covers "js" and "javascript". Elsewhere in this PR (e.g., HTTP adapter) TypeScript aliases are considered.
Improvement: Include "ts" and "typescript" for consistency and to avoid configuration surprises:
switch strings.ToLower(langKey) {
case "python":
normalizedKey = string(evaluatordto.LanguageTypePython)
case "js", "javascript", "ts", "typescript":
normalizedKey = string(evaluatordto.LanguageTypeJS)
}This helps users who author templates with TypeScript labels.
There was a problem hiding this comment.
Aime CR summary: Backend runtime integration (factory/manager + HTTP FaaS adapter) is correct and concurrency-safe. ValidateEvaluator handler correctly restores service invocation. Config decoding and language normalization look good. No Must-Fix or Should-Fix issues found on backend Go changes. A few non-blocking suggestions: centralize default FaaS BaseURL config, remove unused basicSyntaxValidation or wire it into ValidateCode, and consider unifying logs to English for consistency. Full report is available in Feishu: https://bytedance.larkoffice.com/docx/RbfJdgpZsoqiBhxJ6epcqYWXnQf
| name: {{ include "secret.name" . }} | ||
| key: rmq-namesrv-password | ||
| # faas | ||
| - name: COZE_LOOP_PYTHON_FAAS_URL |
There was a problem hiding this comment.
和 docker env 一样的问题，参考其他的 env 写法吧
| port: 8888 | ||
| targetPort: 8888 | ||
|
|
||
| image: |
| custom: | ||
| image: | ||
| registry: | ||
| registry: "" |
| # 后台健康检查循环 | ||
| ( | ||
| while true; do | ||
| if sh /coze-loop/bootstrap/healthcheck.sh; then |
There was a problem hiding this comment.
/coze-loop/bootstrap 和 docker 部署一样路径命名的考虑
| metadata: | ||
| name: {{ include "js-faas.fullname" . }} | ||
| labels: | ||
| {{- include "js-faas.labels" . | nindent 4 }} |
There was a problem hiding this comment.
不要使用换行+nindent 了吧，直接和 4 行一致不换行
| name: coze-loop-python-faas | ||
| description: Python FaaS service for Coze Loop | ||
| type: application | ||
| version: 1.2.0 |
| # 后台健康检查循环 | ||
| ( | ||
| while true; do | ||
| if sh /coze-loop/bootstrap/healthcheck.sh; then |
There was a problem hiding this comment.
仍然 /coze-loop/bootstrap 路径命名
There was a problem hiding this comment.
问题基本和 js-faas 目录下文件一致，参考修改下吧
| port: 8000 | ||
| targetPort: 8000 | ||
|
|
||
| image: |
4 participants
What type of PR is this?
Check the PR title.
(Optional) Translate the PR title into Chinese.
(Optional) More detailed description for this PR(en: English/zh: Chinese).
en:
zh(optional):
(Optional) Which issue(s) this PR fixes: