Skip to content

Case 25- recent search categories (reworked/strengthened prior case 24)#28

Closed
ellierozen wants to merge 8 commits into
ucr-riple:mainfrom
ellierozen:case-25
Closed

Case 25- recent search categories (reworked/strengthened prior case 24)#28
ellierozen wants to merge 8 commits into
ucr-riple:mainfrom
ellierozen:case-25

Conversation

@ellierozen

@ellierozen ellierozen commented May 9, 2026

Copy link
Copy Markdown

Design Dimension
The chosen design dimension for case 25 is D3 Responsibility Decomposition, at a micro level. The case is grounded in the Information Expert principle, which says responsibility for interpreting a piece of data belongs to the class that owns that data. The case goal is to evaluate whether the AI agent will let the consumer code interpret another class's internal data format instead of going through that class's API.

Case goal
The goal of this case is for the agent to implement three new functionalities over the recent-searches buffer. The first is RecentSearches::CountByCategory(category), which returns how many entries belong to a given category. The second is RecentSearches::CategoriesSeen(), which returns the distinct categories present in the buffer in first-appearance order. The third is updating Reporter::Summary so its output includes a per-category count breakdown alongside the existing entry list. The basis of the case is that recent searches are stored as category-prefixed strings (for example, book:harry potter). RecentSearches owns this format because it owns the buffer. The expected design shape keeps format interpretation inside RecentSearches. Other code, especially Reporter, should not parse entries directly; it should call CountByCategory, CategoriesSeen, or a helper method on RecentSearches. There are several acceptable design shapes. The agent can add a public helper like CategoryOf(entry) in RecentSearches and have all three methods use it. The agent can also have Reporter use only the new reader methods and never deal with the format. Furthermore, another option is for the agent to change the storage from vector of strings to to a vector of SearchEntry's, with separate category and term fields, so the consumers never see raw strings.

Evaluator strategy
The evaluator includes both functional tests in recent_searches_test.cc and a structural test in check_separation.py. The functional tests cover edge cases for CountByCategory (empty buffer, single category, multiple categories, missing separator, exact prefix matching) and CategoriesSeen (empty buffer, deduplicated repeats, first-appearance order, missing separator, empty-prefix entries). There is also a Reporter regression test that expects the new bracketed format, and a consistency test that asserts summing CountByCategory with CategoriesSeen() equals the number of valid entries. The structural test enforces information expert in two ways. First, it rejects the ":" separator appearing in any source file outside the format-owning files (recent_searches.cc/.h and search.cc/.h). Second, it requires reporter.cc to call CountByCategory, CategoriesSeen, or CategoryOf, which means Reporter must use the RecentSearches API instead of parsing the entries.

How I addressed the previous reviewer's concerns:

  1. The structural check
    The previous check rejected calls like .find, .substr, .compare, and .starts_with inside CountByCategory, which turned the case into a "blacklist-avoidance puzzle". The new check looks at file-level format ownership: the ":" separator is only allowed in the files that own the format. The agent is free to use any string operation in a properly placed helper. The check also adds a positive assertion that reporter.cc must call the API, which the previous check did not have.

  2. The failure mode is distinct from existing D2/D3 micro cases
    Case 020 (D3 micro) is about architecture: where new packet-assembly logic should live in the module hierarchy. Case 021 (D2 micro) is about reusing a helper that already exists in the repo. Case 25 is about encapsulation: whether an existing class hides its data format from consumers. The principle being tested is named explicitly in the SPEC (Information Expert), and the structural check has a different shape from any existing micro case (file-level format ownership plus a positive API-reach assertion).

  3. The decomposition is necessary
    The previous case had one consumer (CountByCategory), which didn't necessarily need to follow proper separation of responsibilities. The new case has three callers across two files: CountByCategory, CategoriesSeen, and Reporter::Summary. Inlining the parse step means duplicating it across modules, which violates SWE principles. The agent also has to decide where the helper lives so all three callers can reach it, while still maintaining strong cohesion and low coupling.

Rishi-Dave and others added 8 commits May 7, 2026 20:56
…-riple#17)

Strengthens the case 009 (session-expiry-testability) maintainability
check to close two real evasion paths against the D7 testability oracle.
The SPEC at docs/009.session-expiry-testability/SPEC.md:109 explicitly
calls out "system_clock::now(), std::time, or similar wall-clock APIs"
but the check previously only caught the std::* forms.

- FORBIDDEN_SRC_PATTERNS gains POSIX/C wall-clock readers
  (gettimeofday, clock_gettime, gmtime, localtime, mktime) so an agent
  cannot bypass the check by reaching into <sys/time.h> or <ctime>.
- FORBIDDEN_TEST_PATTERNS gains POSIX sleep variants (usleep,
  nanosleep, and bare sleep( with negative-lookbehind to avoid colliding
  with the existing sleep_for/sleep_until matches).
- Adds a positive structural assertion that session_manager.h still
  exposes a SessionManager(..., TimeSource) constructor. Without this
  the seam can disappear silently; the only signal today is a confusing
  link error in the evaluator test.
- Fixes a pre-existing crash: the test-failure message used
  path.relative_to(ROOT) but TEST_DIR lives under EVALUATOR_ROOT, which
  raised a ValueError whenever any test triggered a forbidden pattern.

Adversarially verified: each new pattern fires when its target API is
injected into source/test, and the unmodified starter still passes the
maintainability check.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add case 024.metric-recorder-buffered-flush (D5 micro)

Adds a D5 (Interface and Substitutability Discipline) micro case probing
whether an agent extends an existing abstract base to admit a new
implementation while preserving substitutability for the existing one.

Domain: a metrics subsystem with one immediate-write recorder
(ConsoleMetricRecorder). The agent must add a BufferedMetricRecorder
plus a Checkpoint mechanism that flushes queued metrics through the
abstract MetricRecorder reference, regardless of which concrete
recorder is wired in.

The maintainability pressure forces the agent to decide where in the
type hierarchy the polymorphic Flush() method lives. Putting it only
on the buffered subclass forces the consumer to downcast (breaking
substitutability); putting it on the abstract base with a no-op
override on the immediate-write impl preserves substitutability.

Files added:
- docs/024.metric-recorder-buffered-flush/SPEC.md
- cases/024.metric-recorder-buffered-flush/{TASK.md,CMakeLists.txt,app/main.cc,
  src/{metric_recorder.h,console_metric_recorder.{h,cc},metric_collector.{h,cc}}}
- evaluator/024.metric-recorder-buffered-flush/{checks/check_substitutability.py,
  tests/test_recorder_lifecycle.cc,data/README.md}

CMake registrations:
- cases/CMakeLists.txt: nitr_add_case(024.metric-recorder-buffered-flush)
- evaluator/CMakeLists.txt: nitr_register_case_024 function, dispatcher
  elseif clause, and entry in NITR_BUILD_ALL_CASES nitr_add_evaluator
  enumeration. Both .cc files are compiled into case024_lib (no
  header-only workaround required, addressing PR ucr-riple#13 review feedback).

design_matrix.md: D5 count 2 -> 3, 024 added to assignment table.

The starter compiles cleanly standalone. Like cases 015, 017, 022 and
others, it is intentionally not buildable with the evaluator until the
agent adds BufferedMetricRecorder and the Checkpoint method (matches
the "starter cases are intentionally not buildable until solved"
convention in CLAUDE.md).

Validation (locally, before commit):
- Good-solution probe (abstract base evolved with Flush, both impls
  override, MetricCollector::Checkpoint calls Flush polymorphically):
  python3 tools/run_case.py 024.metric-recorder-buffered-flush
  --with-evaluator builds, all 6 functional gtests pass, and the
  substitutability check prints "Substitutability check passed."
- Bad-solution probe A (Flush only on buffered subclass + dynamic_cast
  in collector): structural check fires assertions 1, 3, 4, 5, 6 as
  expected.
- Bad-solution probe C (no buffered subclass at all, buffering inside
  ConsoleMetricRecorder): structural check fires assertion 2 as
  expected.
- All-cases configure (cmake -DNITR_BUILD_ALL_CASES=ON
  -DNITR_BUILD_EVALUATOR=ON) succeeds end-to-end with 024 wired in.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* 024: tone down TASK.md to let the agent discover the design

Address reviewer feedback on PR ucr-riple#20: the previous "The consumer must be
able to invoke this operation through the same recorder reference it
already uses for Record()" sentence telegraphed that the visibility
trigger should live on the abstract base, defeating the case's
discovery requirement.

Replaces the Flush()-named bullet with the reviewer's suggested vaguer
phrasing ("an explicit visibility trigger ... supporting the checkpoint
scenario below"). The agent must now infer where in the type hierarchy
the trigger belongs from the engineering scenario rather than read it
off TASK.md.

Mirrored the same change in docs/024.../SPEC.md to preserve the
TASK.md-must-be-strict-subset-of-SPEC.md convention.
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.

4 participants