[Feat] Induce helper for some unusual process#1795
Open
Conversation
📝 WalkthroughWalkthroughIntroduces AI skill configuration and documentation for database query assistance and integration test automation, updates test infrastructure documentation and gitignore rules for ProverE2E, and adds a build dependency for libzkp in the coordinator Makefile. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
tests/prover-e2e/report_20260225_1113.txt (1)
1-47: Same concern asreport_20260225_1156.txt— ephemeral test artifact committed to the repo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/prover-e2e/report_20260225_1113.txt` around lines 1 - 47, An ephemeral test artifact (report_20260225_1113.txt, same class as report_20260225_1156.txt) was accidentally committed; remove the file from the repo, stop committing future ephemeral reports by adding a pattern for these report_*.txt files to the repo ignore (e.g., .gitignore), and if needed remove it from the index/history (git rm --cached or purge history) so CI/tests write these reports to a transient location (e.g., /tmp) instead of the source tree; ensure whatever test or script that generates report_*.txt is updated to emit to the transient location or respects the ignore rule.
🧹 Nitpick comments (1)
coordinator/Makefile (1)
37-37:libzkpdependency forces an unconditional clean+rebuild on everylocalsetupinvocation.
libzkpis a.PHONYtarget that always runsclean_libzkp(deletes the.so) before rebuilding. Before this change,coordinator_api: $(LIBZKP_PATH)only rebuilt libzkp when the file was absent. Now everylocalsetupcall triggers a full libzkp recompilation, regardless of whether the.sois already up-to-date.If the intent is to guarantee a fresh libzkp for local setup, this is fine — but it will noticeably increase incremental rebuild time. If the goal was just to ensure libzkp is present, using the file-level target
$(LIBZKP_PATH)directly is cheaper:♻️ Alternative: use the file-based target to avoid unconditional rebuilds
-localsetup: libzkp coordinator_api ## Local setup: build coordinator_api, copy config, and setup releases +localsetup: $(LIBZKP_PATH) coordinator_api ## Local setup: build coordinator_api, copy config, and setup releases🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coordinator/Makefile` at line 37, The localsetup target depends on the phony libzkp target which forces clean_libzkp and a rebuild every run; to avoid unconditional rebuilds change the dependency to the file-level target $(LIBZKP_PATH) (or to coordinator_api which already depends on $(LIBZKP_PATH)) so libzkp is only built when the .so is missing or out-of-date; update the Makefile line with "localsetup: coordinator_api $(LIBZKP_PATH)" or replace "libzkp" with "$(LIBZKP_PATH)" as appropriate, leaving clean_libzkp and the phony libzkp target unchanged if you still need an explicit fresh-rebuild option.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.json:
- Around line 9-14: The settings currently allow "Bash(cat *)" while "deny" is
empty; update the .claude/settings.json deny list to block reading/modifying
sensitive files by adding deny rules such as Read(./.env), Read(./.env.*),
Read(./secrets/**), Read(./**/.ssh/**), Read(./**/*.pem), Read(./**/id_*) and
Write(./.env) (and any other write/modify denies for secrets) so the "Bash(cat
*)" allowance cannot access credentials or keys; locate the "allow" array and
replace the empty "deny" array with these deny entries to explicitly prevent
disclosure of .env, SSH keys, PEMs, secrets folders, and similar sensitive
paths.
In @.claude/skills/db-query/SKILL.md:
- Line 30: The hardcoded output file "query_report.txt" in the psql invocation
(the "-o query_report.txt" usage and reference to /tmp) risks silent overwrites;
change the psql invocation to write to a unique temp filename (use mktemp or
append a timestamp like query_report_$(date +%s).txt) and update any related
references to use that variable, ensuring the skill continues to pass "-w" to
psql and still does NOT read the generated report; update the code that builds
the psql command (where "-o query_report.txt" is inserted) to generate and use
the unique temp path instead.
- Around line 14-16: Fix minor typos and improve phrasing in
.claude/skills/db-query/SKILL.md by replacing "data sheme" with "data schema",
changing "You should have known" to "You should already know", and changing
"use's request" to "user's request"; update the surrounding sentence so it reads
smoothly (e.g., "You should already know the data schema of our database..." and
"According to the user's request, generate the corresponding SQL expression...")
while preserving the examples and intent.
In @.claude/skills/integration-test-helper/SKILL.md:
- Around line 34-60: Fix the typos and the shell-pipe spacing in the SKILL.md
text: replace the token "<desc_of_ccommand>" with "<desc_of_command>", change
"\|tail -n 50" to "| tail -n 50" (add the missing space after the pipe), correct
both occurrences of "target directy" to "target directory", change "error
analyst" to "error analysis", replace "theirself" with "themselves", fix
"mismacthing of configruation" to "mismatching of configuration", and change
"tempoaray path" to "temporary path"; ensure the surrounding sentences (the
lines containing "Use \"| tee <log_file>\"", "Do not read all output, after \"|
tee\"", "We MUST go back to target directory", and the three error-handling
bullets) keep their original meaning while applying these exact textual
corrections.
In `@CLAUDE.md`:
- Around line 3-7: Update the CLAUDE.md text to correct grammar: change "Skills
has been set" to "Skills have been set", change "When asked by" to "When asked",
and change "Now ready for following target" to "Now ready for the following
target"; ensure the listed items (`db-query` and `integration-test-helper` /
`tests/prover-e2e`) and their cost estimates remain unchanged and the
punctuation/spacing around bullets stays consistent.
In `@tests/prover-e2e/.gitignore`:
- Line 5: The .gitignore entry using the broad pattern "*.txt" will silently
ignore any .txt added later; replace it with a narrower, explicit pattern (e.g.,
a specific artifact directory or filename glob like "artifacts/*.txt" or
"build/*.txt") or add explicit negations for tracked .txt fixtures (use
"!docs/*.txt" or similar) so only intended generated/test artifact files are
ignored; update the "*.txt" line in the .gitignore to the chosen specific
pattern.
In `@tests/prover-e2e/experience/ChangePort.md`:
- Line 1: Fix the typo in the ChangePort.md text by replacing "coordiantor" with
"coordinator" (update the sentence that currently reads "Let coordiantor api
listen..." to "Let coordinator api listen..."); do not change meaning or port
number and ensure any corresponding mention in config.json remains consistent.
In `@tests/prover-e2e/README.md`:
- Line 15: The README line about l2.validium_mode requiring
sequencer.decryption_key lacks source guidance; update the README to state
exactly where to obtain that key (e.g., the project’s staff/staging secret
store, the team’s staff-set directory, or by generating it with the project’s
key-generation utility) and include a short note on the expected format and any
steps to retrieve or generate it so developers running validium-mode tests can
find or create sequencer.decryption_key quickly; reference the l2.validium_mode
and sequencer.decryption_key symbols in the added note.
- Around line 19-21: Update the README’s instruction about
sdk_config.coordinator.base_url in config.json to match the actual default port
used in tests and docs: change the example URL from http://localhost:8390 to
http://localhost:18390 (or alternatively add a clear cross-reference to
tests/prover-e2e/experience/ChangePort.md) and mention that reports
(report_20260225_1113.txt and report_20260225_1156.txt) and ChangePort.md use
port 18390 so readers won’t encounter connection failures.
In `@tests/prover-e2e/report_20260225_1156.txt`:
- Around line 1-45: Remove the ephemeral machine-generated run reports
(report_20260225_1156.txt and report_20260225_1113.txt) from the commit and
replace them with a permanent example template (create EXAMPLE_RUN.md) that
contains placeholder fields instead of timestamps/PIDs/temporary paths (e.g.,
replace "/tmp/prover_e2e_logs_20260225_1156.tar.gz" with a placeholder like
"<ARCHIVE_PATH>"); update or confirm .gitignore excludes such test artifacts
(ensure patterns matching *.txt run reports are present) and amend the commit to
only include the EXAMPLE_RUN.md plus the .gitignore change.
---
Duplicate comments:
In `@tests/prover-e2e/report_20260225_1113.txt`:
- Around line 1-47: An ephemeral test artifact (report_20260225_1113.txt, same
class as report_20260225_1156.txt) was accidentally committed; remove the file
from the repo, stop committing future ephemeral reports by adding a pattern for
these report_*.txt files to the repo ignore (e.g., .gitignore), and if needed
remove it from the index/history (git rm --cached or purge history) so CI/tests
write these reports to a transient location (e.g., /tmp) instead of the source
tree; ensure whatever test or script that generates report_*.txt is updated to
emit to the transient location or respects the ignore rule.
---
Nitpick comments:
In `@coordinator/Makefile`:
- Line 37: The localsetup target depends on the phony libzkp target which forces
clean_libzkp and a rebuild every run; to avoid unconditional rebuilds change the
dependency to the file-level target $(LIBZKP_PATH) (or to coordinator_api which
already depends on $(LIBZKP_PATH)) so libzkp is only built when the .so is
missing or out-of-date; update the Makefile line with "localsetup:
coordinator_api $(LIBZKP_PATH)" or replace "libzkp" with "$(LIBZKP_PATH)" as
appropriate, leaving clean_libzkp and the phony libzkp target unchanged if you
still need an explicit fresh-rebuild option.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.claude/settings.json.claude/skills/db-query/SKILL.md.claude/skills/integration-test-helper/ProverE2E.md.claude/skills/integration-test-helper/SKILL.md.gitignoreCLAUDE.mdcoordinator/Makefiletests/prover-e2e/.gitignoretests/prover-e2e/README.mdtests/prover-e2e/experience/ChangePort.mdtests/prover-e2e/report_20260225_1113.txttests/prover-e2e/report_20260225_1156.txtzkvm-prover/.work/.gitignorezkvm-prover/.work/galileo/5fb0b545c66e5670b2aa8856e213d172e843ef2d1350fe43d526856e8340b6476618ea25a7991845bbc5fd571670ee47379ba31ace92d345bca59702a0d4112d/app.elfzkvm-prover/.work/galileo/a03f7324e6e258756c6e705b52cb56651ca01d429273ea5d9892b60dd573ec0622c0e5294bcb1b3a9def836f8d0f18612a9860629b9497292976ca11844b7e73/app.elfzkvm-prover/.work/galileo/bff42a22cd426101b7417d59f421ae0b81bd0a3dd1ace95462a25056f3b5620af2be2f4468d1ac4a70e7682babb1c60417e21c7633d4b55b58f44703ec82b05a/app.elf
Comment on lines
+9
to
+14
| "allow": [ | ||
| "Bash(pwd)", | ||
| "Bash(ls *)", | ||
| "Bash(cat *)" | ||
| ], | ||
| "deny": [] |
There was a problem hiding this comment.
Bash(cat *) with an empty deny list allows reading any file — add deny rules for sensitive paths.
Bash(cat *) is pre-approved without any corresponding deny guard. Anyone running Claude Code in this repo will have it silently cat config files, .env files, SSH keys, or credential files without an interactive prompt. This matters especially for the integration-test-helper skill, which operates on live config directories containing DB credentials and decryption keys (per ProverE2E.md).
.env files, SSH keys, API tokens, and cloud credentials are the priority targets to protect — systematically add deny rules to prevent Claude Code from reading or modifying these files. The official Claude Code docs themselves include deny entries such as Read(./.env), Read(./.env.*), and Read(./secrets/**) as baseline protection examples.
🛡️ Proposed fix: add deny rules for sensitive paths
"permissions": {
"allow": [
"Bash(pwd)",
"Bash(ls *)",
"Bash(cat *)"
],
- "deny": []
+ "deny": [
+ "Bash(cat */.env)",
+ "Bash(cat */.env.*)",
+ "Bash(cat *credentials*)",
+ "Bash(cat *secret*)",
+ "Read(**/.env)",
+ "Read(**/.env.*)",
+ "Read(**/secrets/**)"
+ ]
}📝 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
| "allow": [ | |
| "Bash(pwd)", | |
| "Bash(ls *)", | |
| "Bash(cat *)" | |
| ], | |
| "deny": [] | |
| "allow": [ | |
| "Bash(pwd)", | |
| "Bash(ls *)", | |
| "Bash(cat *)" | |
| ], | |
| "deny": [ | |
| "Bash(cat */.env)", | |
| "Bash(cat */.env.*)", | |
| "Bash(cat *credentials*)", | |
| "Bash(cat *secret*)", | |
| "Read(**/.env)", | |
| "Read(**/.env.*)", | |
| "Read(**/secrets/**)" | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.json around lines 9 - 14, The settings currently allow
"Bash(cat *)" while "deny" is empty; update the .claude/settings.json deny list
to block reading/modifying sensitive files by adding deny rules such as
Read(./.env), Read(./.env.*), Read(./secrets/**), Read(./**/.ssh/**),
Read(./**/*.pem), Read(./**/id_*) and Write(./.env) (and any other write/modify
denies for secrets) so the "Bash(cat *)" allowance cannot access credentials or
keys; locate the "allow" array and replace the empty "deny" array with these
deny entries to explicitly prevent disclosure of .env, SSH keys, PEMs, secrets
folders, and similar sensitive paths.
Comment on lines
+14
to
+16
| You should have known the data sheme of our database, if not yet, read it from the `.sql` files under `database/migrate/migrations`. | ||
|
|
||
| According to use's request, generate the corresponding SQL expression and query the database. For example, if user ask "list the assigned chunks", it means "query records from `chunk` table with proving_status=2 (assigned)", or the SQL expression 'SELECT * from chunk where proving_status=2;'. If it is not clear, you can ask user which col they are indicating to, and list some possible options. |
There was a problem hiding this comment.
Minor typos and phrasing issues.
- Line 14: "data sheme" → "data schema"; "You should have known" → "You should already know"
- Line 16: "use's request" → "user's request"
✏️ Proposed fix
-You should have known the data sheme of our database, if not yet, read it from the `.sql` files under `database/migrate/migrations`.
+You should already know the data schema of our database. If not, read it from the `.sql` files under `database/migrate/migrations`.
-According to use's request, generate the corresponding SQL expression and query the database.
+According to user's request, generate the corresponding SQL expression and query the database.📝 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
| You should have known the data sheme of our database, if not yet, read it from the `.sql` files under `database/migrate/migrations`. | |
| According to use's request, generate the corresponding SQL expression and query the database. For example, if user ask "list the assigned chunks", it means "query records from `chunk` table with proving_status=2 (assigned)", or the SQL expression 'SELECT * from chunk where proving_status=2;'. If it is not clear, you can ask user which col they are indicating to, and list some possible options. | |
| You should already know the data schema of our database. If not, read it from the `.sql` files under `database/migrate/migrations`. | |
| According to user's request, generate the corresponding SQL expression and query the database. For example, if user ask "list the assigned chunks", it means "query records from `chunk` table with proving_status=2 (assigned)", or the SQL expression 'SELECT * from chunk where proving_status=2;'. If it is not clear, you can ask user which col they are indicating to, and list some possible options. |
🧰 Tools
🪛 LanguageTool
[grammar] ~14-~14: Ensure spelling is correct
Context: ...d stop) You should have known the data sheme of our database, if not yet, read it fr...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/db-query/SKILL.md around lines 14 - 16, Fix minor typos and
improve phrasing in .claude/skills/db-query/SKILL.md by replacing "data sheme"
with "data schema", changing "You should have known" to "You should already
know", and changing "use's request" to "user's request"; update the surrounding
sentence so it reads smoothly (e.g., "You should already know the data schema of
our database..." and "According to the user's request, generate the
corresponding SQL expression...") while preserving the examples and intent.
|
|
||
| When you has decided the SQL expression, always print it out. | ||
|
|
||
| You use psql client to query from our PostgreSQL db. When launching psql, always with "-w" options, and use "-o" to send all ouput to `query_report.txt` file under system's temporary dir, like /tmp. You MUST NOT read the generated report. |
There was a problem hiding this comment.
Fixed output path risks silent overwrite on concurrent runs.
query_report.txt under /tmp is a fixed filename. If two developers (or two terminals) invoke the skill simultaneously on a shared machine, the second run will silently clobber the first report. Consider using a timestamped or mktemp-generated filename, e.g., query_report_$(date +%s).txt.
✏️ Proposed fix
-You use psql client to query from our PostgreSQL db. When launching psql, always with "-w" options, and use "-o" to send all ouput to `query_report.txt` file under system's temporary dir, like /tmp. You MUST NOT read the generated report.
+You use psql client to query from our PostgreSQL db. When launching psql, always with "-w" options, and use "-o" to send all output to a uniquely named file under the system's temporary dir, e.g., `/tmp/query_report_<timestamp>.txt` (use the current epoch seconds for the timestamp). You MUST NOT read the generated report.📝 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
| You use psql client to query from our PostgreSQL db. When launching psql, always with "-w" options, and use "-o" to send all ouput to `query_report.txt` file under system's temporary dir, like /tmp. You MUST NOT read the generated report. | |
| You use psql client to query from our PostgreSQL db. When launching psql, always with "-w" options, and use "-o" to send all output to a uniquely named file under the system's temporary dir, e.g., `/tmp/query_report_<timestamp>.txt` (use the current epoch seconds for the timestamp). You MUST NOT read the generated report. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/db-query/SKILL.md at line 30, The hardcoded output file
"query_report.txt" in the psql invocation (the "-o query_report.txt" usage and
reference to /tmp) risks silent overwrites; change the psql invocation to write
to a unique temp filename (use mktemp or append a timestamp like
query_report_$(date +%s).txt) and update any related references to use that
variable, ensuring the skill continues to pass "-w" to psql and still does NOT
read the generated report; update the code that builds the psql command (where
"-o query_report.txt" is inserted) to generate and use the unique temp path
instead.
Comment on lines
+34
to
+60
| + Use "| tee <log_file>" to capture output of bash tool into local file for investigating later. The file name of log should be in format as `<desc_of_ccommand>_<day>_<time>.log` | ||
| + Do not read all output, after "| tee", use "|tail -n 50" to only catch the possible error message. That should be enough for common case. | ||
|
|
||
| It may need to jump to other directories for executing a step. We MUST go back to target directory after every step has been completed. Also, DO NOT change anything outside of target directy. | ||
|
|
||
| ### When error raised | ||
| Command execution should get success return. If error raised while executing, do following process: | ||
|
|
||
| 1. Try to analysis the reason of error, first from the caught error message. If there is no enough data, grep useful information from the log file of whole output just captured. | ||
|
|
||
| 2. Ask User for next action, options are: | ||
| + Retry with resolution derived from error analyst | ||
| + Retry, with user provide tips to resolve the issue | ||
| + Just retry, user has resolved the issue by theirself | ||
| + Stop here, discard current and following steps, do after completion | ||
|
|
||
| Error often caused by some mismacthing of configruation in current host. Here are some tips which may help: | ||
|
|
||
| * Install the missing tools / libs via packet manager | ||
| * Fix the typo, or complete missed fields in configuration files | ||
| * Copy missed files, it may be just put in some place of the project or can be downloaded according to some documents. | ||
|
|
||
| ## After completion | ||
|
|
||
| When every step has done, or the process stop by user, make following materials before stop: | ||
|
|
||
| + Package all log files generated before into a tarball and save it in tempoaray path. Then clear all log files. |
There was a problem hiding this comment.
Fix typos and a piping syntax error in the skill instructions.
As a prompt consumed by Claude, clarity matters. Several typos and one shell-syntax issue are present:
| Line | Current | Fix |
|---|---|---|
| 34 | <desc_of_ccommand> |
<desc_of_command> |
| 35 | |tail -n 50 |
| tail -n 50 (missing space) |
| 37 | target directy (×2) |
target directory |
| 45 | error analyst |
error analysis |
| 47 | theirself |
themselves |
| 50 | mismacthing of configruation |
mismatching of configuration |
| 60 | tempoaray path |
temporary path |
✍️ Proposed fix
-+ Use "| tee <log_file>" to capture output of bash tool into local file for investigating later. The file name of log should be in format as `<desc_of_ccommand>_<day>_<time>.log`
-+ Do not read all output, after "| tee", use "|tail -n 50" to only catch the possible error message. That should be enough for common case.
++ Use "| tee <log_file>" to capture output of bash tool into local file for investigating later. The file name of log should be in format as `<desc_of_command>_<day>_<time>.log`
++ Do not read all output, after "| tee", use "| tail -n 50" to only catch the possible error message. That should be enough for common case.
-It may need to jump to other directories for executing a step. We MUST go back to target directory after every step has been completed. Also, DO NOT change anything outside of target directy.
+It may need to jump to other directories for executing a step. We MUST go back to target directory after every step has been completed. Also, DO NOT change anything outside of target directory.
+ Retry with resolution derived from error analyst
- + Just retry, user has resolved the issue by theirself
+ + Retry with resolution derived from error analysis
+ + Just retry, user has resolved the issue by themselves
-Error often caused by some mismacthing of configruation in current host.
+Error often caused by some mismatching of configuration in current host.
-+ Package all log files generated before into a tarball and save it in tempoaray path. Then clear all log files.
++ Package all log files generated before into a tarball and save it in temporary path. Then clear all log files.🧰 Tools
🪛 LanguageTool
[grammar] ~45-~45: Ensure spelling is correct
Context: ...etry with resolution derived from error analyst + Retry, with user provide tips to resolve...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~50-~50: Ensure spelling is correct
Context: ... completion Error often caused by some mismacthing of configruation in current host. Here ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/integration-test-helper/SKILL.md around lines 34 - 60, Fix
the typos and the shell-pipe spacing in the SKILL.md text: replace the token
"<desc_of_ccommand>" with "<desc_of_command>", change "\|tail -n 50" to "| tail
-n 50" (add the missing space after the pipe), correct both occurrences of
"target directy" to "target directory", change "error analyst" to "error
analysis", replace "theirself" with "themselves", fix "mismacthing of
configruation" to "mismatching of configuration", and change "tempoaray path" to
"temporary path"; ensure the surrounding sentences (the lines containing "Use
\"| tee <log_file>\"", "Do not read all output, after \"| tee\"", "We MUST go
back to target directory", and the three error-handling bullets) keep their
original meaning while applying these exact textual corrections.
Comment on lines
+3
to
+7
| Skills has been set to help some process being handled easily. When asked by "what can you help", list following skills, along with the skill-description and invoke cost estimation here: | ||
|
|
||
| 1. `db-query`: ~$0.1 per query | ||
| 2. `integration-test-helper` Now ready for following target: | ||
| + `tests/prover-e2e`: ~$1.0 per process No newline at end of file |
There was a problem hiding this comment.
Fix grammar issues in the instructions.
A few issues in the CLAUDE.md content that Claude Code loads as context:
- Line 3:
"Skills has been set"→"Skills have been set";"When asked by"→"When asked" - Line 6:
"Now ready for following target"→"Now ready for the following target"
✍️ Proposed fix
-Skills has been set to help some process being handled easily. When asked by "what can you help", list following skills, along with the skill-description and invoke cost estimation here:
+Skills have been set to assist with processes that benefit from contextual guidance. When asked "what can you help", list the following skills along with the skill description and estimated cost:
1. `db-query`: ~$0.1 per query
-2. `integration-test-helper` Now ready for following target:
+2. `integration-test-helper` Now ready for the following target:📝 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
| Skills has been set to help some process being handled easily. When asked by "what can you help", list following skills, along with the skill-description and invoke cost estimation here: | |
| 1. `db-query`: ~$0.1 per query | |
| 2. `integration-test-helper` Now ready for following target: | |
| + `tests/prover-e2e`: ~$1.0 per process | |
| Skills have been set to assist with processes that benefit from contextual guidance. When asked "what can you help", list the following skills along with the skill description and estimated cost: | |
| 1. `db-query`: ~$0.1 per query | |
| 2. `integration-test-helper` Now ready for the following target: | |
| `tests/prover-e2e`: ~$1.0 per process |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 3 - 7, Update the CLAUDE.md text to correct grammar:
change "Skills has been set" to "Skills have been set", change "When asked by"
to "When asked", and change "Now ready for following target" to "Now ready for
the following target"; ensure the listed items (`db-query` and
`integration-test-helper` / `tests/prover-e2e`) and their cost estimates remain
unchanged and the punctuation/spacing around bullets stays consistent.
| conf No newline at end of file | ||
| conf | ||
| *.log | ||
| *.txt No newline at end of file |
There was a problem hiding this comment.
*.txt is broad — may silently swallow future tracked files.
The pattern ignores all .txt files in tests/prover-e2e. If any static test fixture or documentation file with a .txt extension is added later, it will be silently excluded from git without an explicit ! negation. Consider narrowing to the known artifact pattern to make intent explicit:
💡 Proposed narrower pattern
-*.txt
+report_*.txt📝 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
| *.txt | |
| report_*.txt |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/prover-e2e/.gitignore` at line 5, The .gitignore entry using the broad
pattern "*.txt" will silently ignore any .txt added later; replace it with a
narrower, explicit pattern (e.g., a specific artifact directory or filename glob
like "artifacts/*.txt" or "build/*.txt") or add explicit negations for tracked
.txt fixtures (use "!docs/*.txt" or similar) so only intended generated/test
artifact files are ignored; update the "*.txt" line in the .gitignore to the
chosen specific pattern.
| @@ -0,0 +1 @@ | |||
| Let coordiantor api listen at port 18390 to avoid security restriction or port confliction. Also change the corresponding field in `config.json` No newline at end of file | |||
There was a problem hiding this comment.
Typo: "coordiantor" → "coordinator".
✏️ Proposed fix
-Let coordiantor api listen at port 18390 to avoid security restriction or port confliction. Also change the corresponding field in `config.json`
+Let coordinator api listen at port 18390 to avoid security restriction or port confliction. Also change the corresponding field in `config.json`📝 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
| Let coordiantor api listen at port 18390 to avoid security restriction or port confliction. Also change the corresponding field in `config.json` | |
| Let coordinator api listen at port 18390 to avoid security restriction or port confliction. Also change the corresponding field in `config.json` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/prover-e2e/experience/ChangePort.md` at line 1, Fix the typo in the
ChangePort.md text by replacing "coordiantor" with "coordinator" (update the
sentence that currently reads "Let coordiantor api listen..." to "Let
coordinator api listen..."); do not change meaning or port number and ensure any
corresponding mention in config.json remains consistent.
| 6. in `zkvm-prover`, launch `make test_e2e_run`, which would specific prover run locally, connect to the local coordinator api service according to the `config.json`, and prove all tasks being injected to db in step 1. No newline at end of file | ||
| 3. come into `coordinator/build/bin` for following steps: | ||
| + rename `conf/config.template.json` as `conf/config.json` | ||
| + if the `l2.validium_mode` is set to true in `config.json`, the `sequencer.decryption_key` must be set |
There was a problem hiding this comment.
No guidance on how to obtain sequencer.decryption_key.
The README mandates setting sequencer.decryption_key when l2.validium_mode is true but doesn't indicate where this key comes from (e.g., a secret store, the test environment's staff directory, or a generated value). This will block developers running validium-mode tests for the first time.
Consider adding a brief note pointing to the source (e.g., "find this key in the staff set directory under conf/" or similar).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/prover-e2e/README.md` at line 15, The README line about
l2.validium_mode requiring sequencer.decryption_key lacks source guidance;
update the README to state exactly where to obtain that key (e.g., the project’s
staff/staging secret store, the team’s staff-set directory, or by generating it
with the project’s key-generation utility) and include a short note on the
expected format and any steps to retrieve or generate it so developers running
validium-mode tests can find or create sequencer.decryption_key quickly;
reference the l2.validium_mode and sequencer.decryption_key symbols in the added
note.
Comment on lines
+19
to
+21
| + set the `sdk_config.coordinator.base_url` field in `config.json`, so zkvm prover would connect with the locally launched coordinator api, | ||
| for common case the url is `http://localhost:8390` (the default listening port of coordinator api) | ||
| + launch `make test_e2e_run`, which would specific prover run locally, connect to the local coordinator api service according to the `config.json`, and prove all tasks being injected to db in step 1. |
There was a problem hiding this comment.
Default port in README conflicts with ChangePort.md and both test reports.
Line 20 states the default coordinator API URL is http://localhost:8390, but tests/prover-e2e/experience/ChangePort.md explicitly instructs changing the port to 18390, and both report_20260225_1113.txt and report_20260225_1156.txt confirm port 18390 was used in practice. A developer following only the README will likely encounter connection failures.
Either update the default URL here to http://localhost:18390, or add a cross-reference to ChangePort.md at this step.
✏️ Proposed fix
- for common case the url is `http://localhost:8390` (the default listening port of coordinator api)
+ for common case the url is `http://localhost:18390` (see `experience/ChangePort.md` for why port 18390 is used instead of the default 8390)📝 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
| + set the `sdk_config.coordinator.base_url` field in `config.json`, so zkvm prover would connect with the locally launched coordinator api, | |
| for common case the url is `http://localhost:8390` (the default listening port of coordinator api) | |
| + launch `make test_e2e_run`, which would specific prover run locally, connect to the local coordinator api service according to the `config.json`, and prove all tasks being injected to db in step 1. | |
| set the `sdk_config.coordinator.base_url` field in `config.json`, so zkvm prover would connect with the locally launched coordinator api, | |
| for common case the url is `http://localhost:18390` (see `experience/ChangePort.md` for why port 18390 is used instead of the default 8390) | |
| launch `make test_e2e_run`, which would specific prover run locally, connect to the local coordinator api service according to the `config.json`, and prove all tasks being injected to db in step 1. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/prover-e2e/README.md` around lines 19 - 21, Update the README’s
instruction about sdk_config.coordinator.base_url in config.json to match the
actual default port used in tests and docs: change the example URL from
http://localhost:8390 to http://localhost:18390 (or alternatively add a clear
cross-reference to tests/prover-e2e/experience/ChangePort.md) and mention that
reports (report_20260225_1113.txt and report_20260225_1156.txt) and
ChangePort.md use port 18390 so readers won’t encounter connection failures.
Comment on lines
1
to
45
| ProverE2E Integration Test Report | ||
| Date: 2026-02-25 | ||
| Time: 11:56 | ||
| Branch: fix/validium_prover | ||
| conf -> cloak-galileoV2/ (validium mode) | ||
|
|
||
| ## Result: SUCCESS | ||
|
|
||
| All proving tasks completed successfully (exit code 0). | ||
|
|
||
| ## Steps | ||
|
|
||
| ### Step 1: make all | ||
| - DB reset and test data re-imported (make all) | ||
| - Blocks 2–15 fetched from cloak-test-sequencer with validium decryption of 7 L1 messages | ||
| - 4 chunks / 2 batches / 1 bundle inserted into DB | ||
| - testset.json regenerated | ||
|
|
||
| ### Step 2: make coordinator_setup | ||
| - Rebuilt coordinator_api binary with the updated Makefile (localsetup now includes libzkp) | ||
| - Fresh verifier assets downloaded for galileoV2 from S3 | ||
| + root_verifier_vk | ||
| + openVmVk.json | ||
| - genesis.json copied to coordinator/build/bin/conf | ||
|
|
||
| ### Step 3: Restart coordinator_api | ||
| - Old coordinator_api (PID 121277) was killed and restarted with the new binary | ||
| - Listening at port 18390 (per ChangePort experience) | ||
| - Config: l2.validium_mode=true, fork_name=galileoV2, decryption_key set | ||
|
|
||
| ### Step 4: make test_e2e_run | ||
| - Status: SUCCESS (exit code 0) | ||
| - 4 chunk tasks → all proved and submitted successfully | ||
| - 2 batch tasks → all proved and submitted successfully | ||
| - 1 bundle task → proved and submitted successfully | ||
| + EVM proof generated for bundle | ||
| + "All done!" printed by prover | ||
|
|
||
| ## Notes | ||
| - The previous session's circuit mismatch (Feynman vs GalileoV2) was resolved by | ||
| rebuilding coordinator_api via the updated Makefile (libzkp dependency added to localsetup) | ||
| and restarting the coordinator with the rebuilt binary. | ||
|
|
||
| ## Log Archive | ||
| /tmp/prover_e2e_logs_20260225_1156.tar.gz |
There was a problem hiding this comment.
Reconsider committing ephemeral test run reports to the repository.
This file is a machine-generated test run artifact (specific timestamp, specific PID, specific /tmp archive path). Committing dated run logs creates permanent history noise and contains dead references (/tmp/prover_e2e_logs_20260225_1156.tar.gz will never exist anywhere but the original machine). Additionally, the AI summary indicates that .gitignore was updated to exclude .txt artifacts from test runs — yet this file (and its companion report_20260225_1113.txt) are being committed in the same PR, which is inconsistent.
If the intent is to provide a reference example of a successful run, consider converting the content into a permanent EXAMPLE_RUN.md template with placeholder values instead of committing dated artifacts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/prover-e2e/report_20260225_1156.txt` around lines 1 - 45, Remove the
ephemeral machine-generated run reports (report_20260225_1156.txt and
report_20260225_1113.txt) from the commit and replace them with a permanent
example template (create EXAMPLE_RUN.md) that contains placeholder fields
instead of timestamps/PIDs/temporary paths (e.g., replace
"/tmp/prover_e2e_logs_20260225_1156.tar.gz" with a placeholder like
"<ARCHIVE_PATH>"); update or confirm .gitignore excludes such test artifacts
(ensure patterns matching *.txt run reports are present) and amend the commit to
only include the EXAMPLE_RUN.md plus the .gitignore change.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1795 +/- ##
===========================================
+ Coverage 36.41% 36.49% +0.07%
===========================================
Files 248 248
Lines 21321 21321
===========================================
+ Hits 7765 7781 +16
+ Misses 12733 12712 -21
- Partials 823 828 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces AI-powered "skills" to handle tasks that fall outside of our standard daily workflows—such as manual database queries or environment setups for integration testing.
The Problem
Historically, we have automated these processes using a series of scripts. However, these scripts often fail due to environment-specific edge cases when run outside of a developer's local machine. Building a "bulletproof" script to handle every possible edge case is prohibitively costly and leads to significant maintenance overhead.
The Solution
We are now leveraging AI (via Claude Code) to empower these non-standard handlings. By using AI to navigate trivial issues and "read" context, we significantly reduce the manual burden on developers to troubleshoot minor errors during these processes.
This PR adds two initial skills based on Claude Code:
db-query: Execute database queries by providing simple instructions in plain text.Estimated Cost: ~$0.10 per query.
integration-test-helper: Automates the setup and execution of integration tests. The AI parses the README and project structure to perform the necessary steps dynamically.Estimated Cost: ~$1.00 per full prover-e2e test run.
Summary by CodeRabbit
New Features
Documentation
Chores