Stop skills reaching "covered" after two exchanges - #6
Merged
Conversation
Every threshold in the coverage, pacing, portfolio, session, Gemini and preflight paths was hardcoded, so changing e.g. the minimum probe count per skill required a code change and a deploy. Introduce AppConfig: a registry of 29 settings with code defaults, types and ranges, backed by an app_settings table. Values resolve tenant override -> global override -> legacy ENV -> code default, cached in Redis for 30s and invalidated on write, so a change reaches every Puma and Sidekiq process within seconds without a restart. Lookups fall back to the code default if the DB or cache is unavailable. Writes are exposed per tenant via /api/v1/settings (admin) and globally via rake app_config tasks — the API never writes the global scope, so one tenant's admin cannot change another tenant's behaviour. Probe minimums are also embedded in the compiled system prompt, so assessments now store a digest of the config they were compiled under and recompile lazily when it changes. Candidate preflight thresholds (minimum upload/download speed) are served at /api/v1/preflight/config for the frontend device check; the existing /api/v1/speed_test route is unchanged. Defaults match the previous hardcoded values, so behaviour is unchanged until a setting is overridden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A five-skill interview was finishing in ten minutes. Three compounding causes: 1. resolve_state walked forward as many states as Flash proposed, so an initiated skill jumped straight to covered in one analyzer run, using the same probe_count >= 2 check for both hops. Two candidate turns covered a skill, and no skill ever spent a turn at "partial" — the one state whose prompt instructions tell the interviewer to go deeper. Cap advancement at coverage.max_state_advance_per_turn (default 1). 2. "Covered" reused the gate meant for leaving "initiated". Add a separate coverage.min_probes_to_cover (default 4), clamped to never fall below min_probes_to_advance. CoverageAnalyzerWorker's stale- partial path writes covered directly, so it respects the gate too. 3. Coverage completion ended the session regardless of the clock. Hold the wrap-up until session.min_elapsed_ratio_before_wrap_up (default 0.6) of the time limit has elapsed; while held, the coverage map carries wrap_up_blocked and deepen_next so the interviewer pushes the thinnest skill instead of closing. The hold is part of the coverage fingerprint — once every skill is covered the analyzer freezes them, so without it the cache would never refresh and the session would never notice the hold expiring. Also tighten the analyzer's confidence test (evidence must span more than one situation and survive at least one push beyond the first answer) and teach the system prompt the new depth requirement and the wrap_up_blocked / deepen_next fields. All three thresholds are AppConfig settings, so depth can be tuned per tenant without a deploy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The device-check action was named `config`. ActionController inherits a `config` reader from ActiveSupport::Configurable and the rendering stack calls it, so the action shadowed it: every `render` re-entered the action, which rendered again, until the stack ran out. Both endpoints in the controller 500'd, not just that one — the shadowed method is controller-wide, so `speed_test` died the same way. The candidate device check reported "Internet: Failed, 0 Mbps, 999 ms" with a perfectly good connection, and each recursion burned ~1.2M allocations, pinning both Puma workers at 90% CPU until every other request crawled (a 1-row `SELECT ... LIMIT 1` logged at 807ms, measured at 1.1ms). Rename the action to `thresholds`; the route path is unchanged, so the frontend needs no change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A fresh database left you unable to log in: seeds created the org and the skill taxonomy but no user, while POST /auth/login looks a row up and demands role 'admin'. The closing instructions still described minting a JWT by hand, which predates the frontend login flow. Adds an idempotent admin (admin@test.com / password123 — password is left alone if the row already exists), plus a 30-minute Backend Engineer assessment and a matching vacancy, both built from five taxonomy skills so their level anchors stay in sync with B7_SKILLS. The system prompt is compiled inline rather than through SystemPromptGeneratorWorker: the compiler is pure string interpolation, so seeding stays independent of Redis and Sidekiq. tenant_id is passed explicitly on every create. TenantScoped normally fills it from Current.tenant_id, which is set per request from the JWT, and there is no request here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The depth gates added in f52a846 govern when the analyzer may call a skill covered. They cannot stop the model from deciding on its own that the conversation is over, and it does: a test run closed at 18:37 while the coverage map injected seconds earlier still read skills_remaining=1, with Testing & QA at two probes. finalize_natural_close ended the session and hardcoded reason 'all_covered', so the premature ending was invisible in the data. Same failure as before, one layer further out. Refuse that close. When the model says goodbye with configured skills uncovered, inject a signal naming the open skills instead of finalizing — told only "keep going" it re-closes on the next turn. The guard sits on both close paths: handle_coverage_auto_end and the closing-phrase fallback timer, since a skipped turnComplete would otherwise walk the very close this guard exists to stop through the back door. Bounded by session.max_premature_close_pushbacks (default 2) so an insistent model cannot deadlock the session against the candidate's patience, and skipped once the wrap-up time warning has been sent, where the clock outranks the agenda. Coverage flags are refreshed even when the coverage fingerprint is unchanged, and seeded on connect. Once every skill is covered the analyzer freezes them and the digest stops moving; a reconnect starts a fresh ConnectionState. Without both, the guard would silently disarm itself for the rest of the session. end_reason stops lying: new value 'ended_early' for a session that stopped with the agenda open. audio_complete still always ends the session — coverage decides the reason, never whether to end — so the false negatives that once stalled auto-end cannot return. Discovered skills are excluded from the judgement; they are a by-product of the conversation, not something the interview promised to assess. Not yet exercised against a live model: whether Gemini honours the continue signal, and whether two pushbacks is the right ceiling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guard shipped in 8114f4f fired correctly on a live interview and then made a mess of it: three AI turns in ten seconds, two of them farewells, and the session ended right after the interviewer finally asked its question — before the candidate could answer. Three faults, all in the guard: 1. inject_context makes Gemini generate immediately, and a model mid- closing repeats the goodbye. Steering that repeat generated another, and another. Steer at most once per exchange now, tracked by whether the candidate has actually spoken since — the first attempt counted turn_counter, which advances for both speakers, so two consecutive AI turns cleared it. 2. A compliant turn was counted as a close. The interviewer obeyed and asked about the open skill, but opened with "I think that gives me a clear picture", which matched the closing phrases and spent budget. A turn ending in a question is a bridge, not a farewell. 3. The closing-phrase fallback timer outlived the refusal that should have killed it. Armed by the first close, it fired 15s later with the pushback budget spent and ended the session. It is cancellable now, replaced on each closing phrase and cancelled whenever a close is refused. Refusing without re-injecting opened a way to hold the session hostage: the candidate goes quiet believing it is over, the silence pump keeps waking the model, and it keeps saying goodbye against a guard that never concedes. Bounded by the same pushback limit, so the worst case is the farewell plus two model-driven repeats rather than a wait for the time ceiling. Verified by unit exercise, not against a live model: interview start with an empty coverage map raises nothing and cannot trip the guard, and five consecutive closes with a silent candidate produce exactly one injection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assessment and vacancy indexes take a `q` filter and carry their own counts (skills, sessions, fit/gap reports) so the console does not need a request per row. A vacancy now shows which candidates have been analysed against it, newest first and capped, with match and gap tallies. Adds a cross-session GET /fit_gap_reports index for the same console. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # app/channels/audio_websocket_middleware.rb # app/controllers/api/v1/preflight_controller.rb # app/lib/app_config.rb # app/services/assessments/system_prompt_compiler.rb # app/services/coverage/analyzer.rb # app/services/coverage/state_engine.rb # app/workers/coverage_analyzer_worker.rb # config/routes.rb # db/schema.rb
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.
Stacked on #5 (base:
feat/runtime-config) — merge that first.Problem
A 5-skill interview was finishing in ~10 minutes: roughly 2 minutes per skill. Three causes compound.
1. A skill could jump
initiated→coveredin a single turnresolve_statewalked forward as many states as Flash proposed, applying the sameprobe_count >= 2check at every hop:So when Flash proposed
coveredfor a skill sitting atinitiated, the loop passedinitiated→partial(probe 2 ≥ 2 ✓) and then immediatelypartial→covered(probe 2 ≥ 2 ✓). Two candidate turns covered a skill. The analyzer runs once per candidate turn and capsprobe_countat +1 per run, so 5 skills × 2 turns ≈ 10 minutes — exactly what was observed.The second-order effect is worse than the arithmetic: no skill ever persisted in
partial, so the coverage map never once told the interviewer "partial — you've probed but signal is still thin. Go deeper. Do NOT wrap up." Everypartialinstruction in the system prompt was dead code in practice.2.
coveredreused the gate meant for leavinginitiatedcoverage.min_probes_to_advance(2) was the only depth requirement anywhere. Nothing expressed "this skill has been probed enough to be done".Corroborating symptom: the portfolio grades
highconfidence atprobe_count >= 3, so with every skill finishing at 2 probes, no skill could ever be rated high confidence.3. Coverage completion ended the session regardless of the clock
all_covered?triggered wrap-up as soon as the agenda was complete. A generous analyzer could therefore close a 45-minute interview at minute 15 and leave 30 minutes unused.Fix
1. Cap advancement at one state per analyzer run — new
coverage.max_state_advance_per_turn(default 1). A skill must sit atpartialfor at least one turn, which is the only point at which the interviewer is told to go deeper on it. Walking forward one step (rather than rejecting a multi-step proposal outright) still prevents the "stuck state" case the original loop was written for; it just costs one extra turn.2. Separate gate for
covered— newcoverage.min_probes_to_cover(default 4), clamped to never fall belowmin_probes_to_advancewhatever the config says.CoverageAnalyzerWorker#advance_stale_partialswritescovereddirectly without going throughStateEngine, so it now applies the same floor.3. Hold the wrap-up while time remains — new
session.min_elapsed_ratio_before_wrap_up(default 0.6). While held, the coverage map carrieswrap_up_blocked: trueanddeepen_next: <thinnest skill id>, and the system prompt instructs the interviewer to push that skill harder rather than close — looking for the ceiling of the candidate's ability, not a recap. Set to0to restore the old behaviour.4. Tighter confidence test in the analyzer prompt — evidence must span more than one situation (not the same story retold) and the candidate must have been pushed at least once beyond their first answer. Unsure →
partial.5. System prompt — states the new depth requirement with concrete follow-up shapes, explains
wrap_up_blocked/deepen_next, and adds the one exception to "never ask about a covered skill".Verification
Turn-by-turn simulation through
StateEngine(defaults: advance 2, cover 4, max advance 1):not_yetinitiatedinitiatedinitiatedinitiatedcoveredcovered← the bugpartialpartialcoveredpartialpartialcoveredcoverednot_yetcoveredcoveredinitiatedWrap-up hold, 45-minute limit, all skills covered: 10m → held, 20m → held, 27m → releases. Payload while held carries
wrap_up_blocked: true, deepen_next: "s1"(the skill with the lowest probe count). Fingerprint verified to change when the hold expires. Tenant overridemin_probes_to_cover = 6verified: probe 4 and 5 staypartial,coveredat 6.Impact on interview length
Roughly
skills × min_probes_to_covercandidate turns at ~1 minute each — 5 skills ≈ 20+ minutes at the defaults, against ~10 before.time_limit_minon short assessments. A 10-minute assessment with 5 skills cannot fit 20 turns, so no skill reachescoveredand the session ends on the time ceiling instead. That is safe — no hang, transcript and portfolio still generate — but every skill will be rated low/medium confidence. For short assessments lowercoverage.min_probes_to_coverfor that tenant, or configure fewer skills.All three thresholds are
AppConfigsettings, so the depth can be tuned per tenant at runtime without a deploy — 3 vs 4 vs 5 can be compared on staging directly.🤖 Generated with Claude Code