feat(job-analysis): add governed qualification-rule review - #104
feat(job-analysis): add governed qualification-rule review#104seonghobae wants to merge 13 commits into
Conversation
|
Warning Review limit reachedNext included review available in 46 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Current-head correction: production implementation is now present at |
|
Current-head evidence update: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return json.dumps(payload, sort_keys=True, separators=(",", ":"), ensure_ascii=True) | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True, weakref_slot=True, eq=False, repr=False) |
There was a problem hiding this comment.
📝 Info: Tamper detection relies on identity-based hash
The packet uses eq=False, so hashing and equality fall back to object identity. This is what lets the weak-key issuance store still find the original digest after a field is mutated, making the post-issuance tamper check work (review.py:298-306). Switching to eq=True or a field-based hash would break that guarantee.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if recorded_at < reviewed_at: | ||
| raise ValueError("recorded_at cannot precede reviewed_at") |
There was a problem hiding this comment.
📝 Info: Future-review guard tied to construction wall clock
The only guard against a future review time is recorded_at < reviewed_at (review.py:255), where recorded_at is now() at construction. Clock skew that makes a reviewer timestamp slightly ahead of the issuing host will reject an otherwise valid review.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@opencode-agent Please review the current unchanged head against protected |
…n-subclassable Strix OpenAI-direct scan of head d92ac4c surfaced one MEDIUM finding: a hostile in-process subclass could override _validated_payload to bypass all trust-bearing field validation. Repair: __init_subclass__ now raises TypeError for every subclass, so the trust boundary is non-overridable by construction. Regression coverage asserts subclass definition fails closed at class-definition time and base-class canonical evidence stays stable per issued instance. 55 tests, 100% owned statement/branch coverage.
|
|
||
| assert first.canonical_json() == first.canonical_json() | ||
| assert first.sha256_digest() == first.sha256_digest() | ||
| assert second.sha256_digest() != first.sha256_digest() |
There was a problem hiding this comment.
📝 Info: Digest inequality test relies on clock granularity
test_valid_packets_still_canonicalize_and_hash_deterministically asserts two packets built from identical inputs hash differently. The only differing field is recorded_at from datetime.now(timezone.utc) (review.py). If two constructions share a clock tick the digests match and the test fails. Microsecond resolution makes this practically unreachable on Linux, but the assertion depends on timer granularity, not a deterministic difference.
Was this helpful? React with 👍 or 👎 to provide feedback.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| with pytest.raises(TypeError, match="must not be subclassed"): | ||
|
|
||
| class ForgedPacket(JobQualificationRuleReviewPacket): |
1 participant
Buyer-visible gap
Protected
develop@9e3e4847510e1e612b48474ba42b177b8ed824dfmodels Job Analysis Task/FJA/KSAO evidence and the PRD requires qualification rules, but it has no bounded governance evidence proving which Job Analysis evidence supports a proposed qualification rule before that rule can influence recruiting or selection.This Orgmetra-only slice adds a human-reviewed, PII-minimized qualification-rule review boundary. It does not evaluate candidates, reject applicants, mutate Job/Job Analysis, or write any foreign CWL repository.
Test-first repair sequence
f06498079027b0f381beaf500ba7487ddcd6e555introduced the package contract, adversarial regressions, exact-head workflow, ADR, APA-7 doctoring, traceability, README and CHANGELOG while productionreview.pywas absent. No predecessor/superseded run is promoted as current evidence.JobQualificationRuleReviewPacketwith exact runtime primitives, authoritative non-sentinel Job/Job Analysis references, packet-owned UUIDv4 artifact/actor correlations, lower-case SHA-256 Job Analysis/rule/Task/KSAO/source provenance, controlled non-sensitive rule categories and reasons, business-effective date, explicit evidence version, distinct requester/reviewer, human review time and owner-generated system-recorded UTC issuance time.d92ac4cb798b3bd32b632c0ab677c03f944070e4additionally fixes the malformed-but-correctly-namespaced Job reference boundary:job_record:not-a-uuidfails closed rather than escaping the governed validation path.Current contract
Canonical evidence is fixed to
job_qualification_rule_review, mandatory human review,reviewed_for_authoritative_resolution, andnot_authorized_for_candidate_or_employment_decision. It excludes candidate/person PII, candidate qualification outcomes, assessment/cut scores, compensation, raw qualification-rule text, prompts and model output.Before an authoritative rule is persisted or used in recruiting/selection, the host must re-resolve the exact tenant/Job/Job Analysis snapshot, qualification-rule artifact, Task/KSAO/source provenance and accountable reviewer authority at the relevant business-effective coordinate, then preserve immutable audit/outbox evidence. The packet itself grants no candidate or employment-decision authority.
Trust-bearing text, dates, timestamps and evidence versions require exact runtime primitives; Nil/Max UUID sentinels, malformed/noncanonical references, malformed SHA-256, actor overlap, unreviewed categories/reasons, future human review relative to issuance, mutated governance constants and post-issuance evidence mutation fail closed. Canonical exports are generated from a verified evidence snapshot against the creation-time issuance digest.
Research boundary
OPM job-analysis and qualification-policy guidance plus the Uniform Guidelines on Employee Selection Procedures are recorded as design evidence only. This package makes no legal-compliance, validity, qualification, or certification determination.
Exact-current-head evidence
Base:
develop@9e3e4847510e1e612b48474ba42b177b8ed824dfHead:
d92ac4cb798b3bd32b632c0ab677c03f944070e4Every applicable exact-current-head workflow is terminal GREEN:
32671485522— success32671485458— success32671485471— success32671485492— success32671485484— successFresh review state has one
COMMENTEDDevin review and two unresolved informational threads: identity-based hashing is deliberately required by the process-local weak issuance registry so field mutation cannot change lookup identity, and the review/issuance chronology guard intentionally fails closed when reviewer time is ahead of the issuing host clock. Neither thread identifies a current authorization, privacy, data-integrity, or canonical-evidence bypass, so they remain unresolved rather than being falsely marked addressed. There is no qualifying independent non-authorAPPROVEand noCHANGES_REQUESTEDreview. GitHub reports the PR open, ready-for-review and mergeable.Merge governance
Ready-for-review is not merge authorization. Keep unmerged until a qualifying independent non-author approval exists on this unchanged head and issue #89's enforceable
developprotection defect is actually repaired. Immediately before any future merge, refetch exact head, live base, reviews, unresolved threads, effective rules/protection and exact-current-head checks and use expected-head protection only if every live gate is satisfied. Do not self-approve, weaken or simulate protection, manufacture passing evidence, or transfer predecessor checks/reviews.