[agent/gemini] Developer experience audit. Check: 1) CLAUDE.md quality — ... #1

Closed
Virgil wants to merge 1 commit from agent/developer-experience-audit--check--1--cl into main

37
FINDINGS.md Normal file
View file

@ -0,0 +1,37 @@
# Developer Experience Audit Findings for core/config
This report summarizes the findings of a developer experience audit conducted on the `core/config` package.
## 1. CLAUDE.md Quality
**Finding:** The `CLAUDE.md` file is comprehensive and well-written. It clearly explains the package's architecture, conventions, and dependencies.
**Issue:** The build and test commands listed in `CLAUDE.md` are outdated. Commands like `core go test` and `core go cov` do not work. This is a significant issue for new developers trying to get started with the project.
## 2. Test Coverage
**Finding:** The test coverage for the package is **67.5%**.
**Issue:** This coverage percentage is somewhat low. A healthy target for a mature and critical package like this would typically be above 80-85%. Increasing test coverage would improve the robustness of the package.
## 3. Error Handling Conventions
**Finding:** The project correctly avoids using `fmt.Errorf`, adhering to the convention of using `coreerr.E` from the `go-log` package.
**Issue:** There are five instances of `fmt.Sprintf` being used to format the error message passed to `coreerr.E`. While not a direct violation of the "no `fmt.Errorf`" rule, it is redundant as `coreerr.E` likely supports variadic arguments for formatting. This adds unnecessary complexity and verbosity to the code.
## 4. File I/O Conventions
**Finding:** The package correctly uses the `go-io` `Medium` abstraction for all file reading and writing operations, avoiding direct calls to functions like `os.ReadFile` or `os.WriteFile`.
**Issue:** The `New()` function in `config.go` and a test in `config_test.go` use `os.UserHomeDir()` to determine the default path for the configuration file. This is a minor issue, but it does mean that the default configuration path is dependent on the host environment and is not fully abstracted, which could cause issues in certain sandboxed or containerized environments.
## Recommendations
1. **Update `CLAUDE.md`:** The build, test, and coverage commands in `CLAUDE.md` should be updated to reflect the current state of the `core` CLI tool. The correct command to run tests with coverage was found to be `go test -cover forge.lthn.ai/core/config`. If a `core` command is preferred, its correct syntax should be documented.
2. **Improve Test Coverage:** Add more unit tests to increase the test coverage from 67.5% to over 80%. This will increase confidence in the package's correctness.
3. **Refactor Error Handling:** Remove the redundant `fmt.Sprintf` calls when creating errors with `coreerr.E`. For example, `coreerr.E("config.Get", fmt.Sprintf("key not found: %s", key), nil)` should be changed to something like `coreerr.E("config.Get", "key not found: %s", key)`.
4. **Abstract Home Directory Logic (Optional):** For full abstraction, consider passing the home directory or the full default path into the `New` function, rather than having the constructor call `os.UserHomeDir()` itself.