Skip to content

fix: reject directory args in keywords CLI - #167

Merged
cardmagic merged 2 commits into
masterfrom
fix/keywords-directory-args-and-hound-config
Aug 15, 2026
Merged

fix: reject directory args in keywords CLI#167
cardmagic merged 2 commits into
masterfrom
fix/keywords-directory-args-and-hound-config

Conversation

@cardmagic

Copy link
Copy Markdown
Owner

Fast follow to #163. Closes the one review item that did not land before the merge, and stops the Hound noise.

1. Directory arguments leaked a raw errno

command_fit globbed arguments without a file check, and command_extract only checked File.exist?. A directory reached File#read and surfaced the errno through the generic StandardError rescue:

$ keywords fit -m d.json corpus
Error: Is a directory @ io_fillbuf - fd:6 /path/corpus     # exit 1

$ keywords extract corpus
Error: Is a directory @ io_fread - /path/corpus            # exit 1

Every other input error in the tool exits 2 with a clear message, so this path was the odd one out.

It also broke the workflow the README leads with. A shell expands keywords fit corpus/* before the CLI sees it, so a subdirectory arrives as its own argument and the whole run fails:

$ keywords fit corpus/*        # corpus/ holds a.txt, b.txt, subdir/
Error: Is a directory @ io_fillbuf - fd:6 /path/corpus/subdir    # exit 1

Behavior now

Command Before After
keywords fit corpus/* (subdir present) errno, exit 1 fits the files, exit 0
keywords fit corpus errno, exit 1 Error: No files to fit, exit 2
keywords extract corpus errno, exit 1 Error: "..." is a directory, not a file, exit 2
keywords fit corpus/a.txt corpus/NOPE.txt No files matched, exit 2 unchanged
keywords fit 'corpus/*.md' No files matched, exit 2 unchanged

fit skips directories and keeps the files around them. A path that matches nothing still fails, so typos are still caught. An argument set that yields no file at all fails with No files to fit.

This also removes a split introduced by the per-argument guard in #163: a quoted 'corpus/*' and an unquoted corpus/* behaved differently, because the CLI expands the first itself. Both now do the same thing.

2. Hound reported 96 offenses that CI does not see

Hound flags Metrics/LineLength. RuboCop renamed that cop to Layout/LineLength in 0.78 (December 2019). Hound also flags at stock defaults (LineLength 80, MethodLength 10, ClassLength 100, AbcSize 15) while .rubocop.yml sets 140, 25, 250, and 30 and excludes this file. It flags Style/Documentation and Style/FrozenStringLiteralComment too, which .rubocop.yml disables outright.

So Hound never read .rubocop.yml. This adds .hound.yml to point it there.

Note that Hound may still not honor it. Its RuboCop is too old to parse plugins: (needs 1.72+) or NewCops (needs 0.90+), so it can fall back to defaults again. If the noise continues, the next step is to disable the linter with ruby: {enabled: false} or remove the Hound app, since the CI lint job already runs RuboCop 1.89 against .rubocop.yml on every PR and is the real gate.

Verification

Tests written first, confirmed failing with exit 1, then fixed.

  • 709 runs, 0 failures, 1 skip
  • bundle exec rubocop: 56 files, no offenses
  • rbs-inline + rbs validate: pass
  • Built the gem, installed it into an isolated GEM_HOME, and drove the real keywords binary through the table above plus 13 regression checks from the feat: add keywords CLI tool for text vectorization (#122) #163 review rounds: bigram labels, min_word_length models, snake_case tokens, missing-model guards on transform and info, empty stdin, negative -n, --min-df/--max-df/--ngram validation, and the 600-file run under ulimit -n 256. All hold.

Four tests added: glob with a subdirectory, bare directory to fit, directory to extract, and a nonexistent path among valid ones as a regression guard.

`keywords fit` and `keywords extract` passed directories straight to
File#read, so users got a raw errno and exit 1:

    $ keywords fit corpus
    Error: Is a directory @ io_fillbuf - fd:6 /path/corpus

Every other input error in the tool exits 2 with a clear message. This
one path did not. It also broke the README's headline workflow, because
a shell-expanded `keywords fit corpus/*` sends any subdirectory as its
own argument.

`fit` now skips directories and keeps the files around them, so
`corpus/*` works when `corpus/` holds a subdirectory. A path that
matches nothing still fails, so typos are still caught. An argument set
that yields no file at all fails with "No files to fit". `extract`
rejects a directory outright.

This also removes a split where a quoted `'corpus/*'` and an unquoted
`corpus/*` behaved differently, since the CLI expands the first itself.

Add .hound.yml pointing Hound at .rubocop.yml. Hound reported 96
offenses on #163 that CI does not see, because it flags `Metrics/
LineLength` (renamed to `Layout/LineLength` in RuboCop 0.78) at stock
defaults, and it flags cops this repo disables.

@houndci-bot houndci-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

.rubocop.yml: Layout/LineLength has the wrong namespace - should be Metrics
.rubocop.yml: Layout/LineLength has the wrong namespace - should be Metrics
Warning: unrecognized cop plugins found in .rubocop.yml
Warning: unrecognized cop Naming/MethodParameterName found in .rubocop.yml
Warning: unrecognized cop Lint/UselessConstantScoping found in .rubocop.yml
Warning: unrecognized cop Minitest/MultipleAssertions found in .rubocop.yml
Warning: unrecognized cop Style/OneClassPerFile found in .rubocop.yml
Error: Unknown Ruby version 3.1 found in `TargetRubyVersion` parameter (in .rubocop.yml).
Supported versions: 2.1, 2.2, 2.3, 2.4, 2.5

@greptile-apps

greptile-apps Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR normalizes directory handling in the keywords CLI and configures Hound to use the repository’s RuboCop settings.

  • Filters directories from fit inputs while preserving unmatched-path errors and rejecting an empty resulting file set.
  • Rejects directory arguments to extract with a usage error.
  • Adds regression coverage for directory and missing-path behavior.
  • Adds .hound.yml pointing Hound at .rubocop.yml.

Confidence Score: 5/5

The PR appears safe to merge with no actionable regressions identified.

The changed CLI paths correctly exclude directories from fitting, reject directory extraction with the intended usage error, preserve unmatched-path failures, and include focused regression coverage.

Important Files Changed

Filename Overview
lib/classifier/keywords/cli.rb Adds regular-file filtering for fit inputs and explicit directory rejection for extract without introducing an actionable defect.
test/keywords/cli_test.rb Covers mixed file/directory globs, directory-only fit input, missing fit paths, and directory extract input.
.hound.yml Enables Hound’s Ruby linter and directs it to the existing RuboCop configuration.

Reviews (1): Last reviewed commit: "fix: reject directory args in keywords C..." | Re-trigger Greptile

Pointing Hound at .rubocop.yml made it read the file, but its RuboCop
cannot parse it:

    .rubocop.yml: Layout/LineLength has the wrong namespace
                  - should be Metrics

Hound bundles a RuboCop older than 0.78, which renamed that cop in
December 2019. The same version also rejects TargetRubyVersion 3.1.

One file cannot serve both. RuboCop 1.89 refuses Metrics/LineLength as
obsolete, and Hound refuses Layout/LineLength, so any edit that satisfies
Hound breaks the CI lint job.

A translated legacy config would need its own copy of every maximum and
exclusion, would drift from .rubocop.yml, and would still judge Ruby 3.1
code and rbs-inline annotations by 2019 cops.

Disable the Ruby linter instead. CI runs RuboCop 1.89 against
.rubocop.yml on every pull request and is the real gate. Hound still
lints other file types.
@cardmagic
cardmagic merged commit b6b012b into master Aug 15, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants