Delete dead code and unify CSV escaping - #14
Merged
Conversation
agents list --format csv reimplemented the CSV quoting rule and dropped two things the canonical writer has: null handling, and the guard against a leading =, +, - or @ being executed as a spreadsheet formula. One CSV path was defused and the other was not, which is the predictable result of a second copy. Both now go through CsvWriter.EscapeField, and a test asserts the two paths agree field for field so they cannot drift again. Removed as genuinely unreferenced: - DownloadHandleWire, whose full-result download endpoints are not implemented at all, so it modelled a response nothing can receive. - DatabricksProfiles.Find, public API of a shipped library that nothing calls and that Load().FirstOrDefault replaces in one line. Unused public surface is surface supported forever without ever learning whether its shape is right. 135 tests green.
Both comments were mine, and both asserted a property the code does not have — the same class of defect this project has now hit three times, which is why the review brief targets it specifically. GenieAuthenticationHandler claimed 'no call site handles a raw token'. AuthCommand does exactly that, at line 47, to report the token's length. The real invariant is narrower: this handler is the only place in the HTTP pipeline that touches a raw token, so no request-building code can put one in a URL or a log by accident. ExitCode.From claimed an unmapped failure kind is 'a bug to catch at compile time'. The switch has a discard arm, which makes it exhaustive to the compiler, so a new enum member compiles silently and CS8509 never fires. The gap is caught by ExitCodeTests.Every_failure_kind_is_mapped in CI — the comment now says so. Docs: CHANGELOG still said nothing had been verified live, contradicting both the README and compatibility.md; two ROADMAP boxes described finished work; a blank line inside compatibility.md's table would have rendered three rows as literal pipes on GitHub; and commands.md called itself the complete reference while omitting export and feedback — two of the eight registered commands, and two the compatibility matrix cites as live-tested. The pack-run row now records that one of its two answers came back as a Genie clarifying question rather than a clean answer, which is what the evidence shows.
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
1 participant
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.
agents list --format csvreimplemented the CSV quoting rule and dropped two things the canonical writer has: null handling, and the guard against a leading=,+,-or@being executed as a spreadsheet formula. One CSV path was defused against formula injection and the other was not — the predictable result of a second copy. Both now go throughCsvWriter.EscapeField, and a test asserts the two paths agree field for field so they cannot drift again.Removed as genuinely unreferenced (verified by grep across
src/andtests/):DownloadHandleWire— the full-result download endpoints it belongs to are not implemented at all, so it modelled a response nothing can receive.DatabricksProfiles.Find— public API of a shipped library that nothing calls, replaced in one line byLoad().FirstOrDefault. Unused public surface is surface supported forever without ever learning whether its shape is right.135 tests green.