fix(session_metrics): guard VIBEGUARD_LOG_FILE against KeyError (#103)#105
Open
majiayu000 wants to merge 2 commits intomainfrom
Open
fix(session_metrics): guard VIBEGUARD_LOG_FILE against KeyError (#103)#105majiayu000 wants to merge 2 commits intomainfrom
majiayu000 wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Hook Latency (P95)
Details
| Benchmark suite | Current: 9bec1ad | Previous: 075c3ed | Ratio |
|---|---|---|---|
pre-edit-guard (P95) |
184 ms |
188 ms |
0.98 |
pre-write-guard (P95) |
221 ms |
213 ms |
1.04 |
pre-bash-guard (P95) |
259 ms |
255 ms |
1.02 |
post-edit-guard (100) (P95) |
296 ms |
298 ms |
0.99 |
post-write-guard (100) (P95) |
209 ms |
208 ms |
1.00 |
post-edit-guard (5000) (P95) |
315 ms |
314 ms |
1.00 |
post-write-guard (5000) (P95) |
207 ms |
209 ms |
0.99 |
stop-guard (5000) (P95) |
132 ms |
132 ms |
1 |
learn-evaluator (5000) (P95) |
131 ms |
132 ms |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
…ssing env var Replace bare os.environ["VIBEGUARD_LOG_FILE"] with .get() + early sys.exit(0) so invocations without the variable exit silently instead of crashing with a KeyError traceback, matching the existing guards for VIBEGUARD_SESSION_ID and VIBEGUARD_PROJECT_LOG_DIR added in PR #93. Add two test cases to test_session_metrics_env_guard.sh covering exit-0 and no-metrics-file-written behaviour when VIBEGUARD_LOG_FILE is unset. Signed-off-by: majiayu000 <1835304752@qq.com>
Assert that the missing VIBEGUARD_LOG_FILE guard exits silently, and reset the vg-helper session-metrics test temp dirs so repeated runs do not accumulate stale JSONL state. Signed-off-by: majiayu000 <1835304752@qq.com>
8677a28 to
9bec1ad
Compare
Owner
Author
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
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
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.
Summary
os.environ["VIBEGUARD_LOG_FILE"]at line 35 ofhooks/_lib/session_metrics.pywith.get()+sys.exit(0)guard, matching the pattern used forVIBEGUARD_SESSION_IDandVIBEGUARD_PROJECT_LOG_DIRadded in PR fix(session_metrics): guard env vars against silent KeyError crash #93tests/unit/test_session_metrics_env_guard.shcovering exit-0 and no-metrics-file-written behaviour whenVIBEGUARD_LOG_FILEis unset (6/6 pass)Closes #103
Test plan
bash tests/unit/test_session_metrics_env_guard.sh— all 6 tests pass (4 existing + 2 new)python3 hooks/_lib/session_metrics.pywithoutVIBEGUARD_LOG_FILEset → exits 0 with no tracebackSigned-off-by: majiayu000 1835304752@qq.com